paths: terminate recursive mkdir walk when a confirmed parent still yields ENOENT - #36162
Open
robobun wants to merge 13 commits into
Open
paths: terminate recursive mkdir walk when a confirmed parent still yields ENOENT#36162robobun wants to merge 13 commits into
robobun wants to merge 13 commits into
Claude / Claude Code Review
completed
Jul 28, 2026 in 12m 33s
Code review found 1 important issue
Found 2 candidates, confirmed 2. See review comments for details.
Details
| Severity | Count |
|---|---|
| 🔴 Important | 1 |
| 🟡 Nit | 1 |
| 🟣 Pre-existing | 0 |
| Severity | File:Line | Issue |
|---|---|---|
| 🔴 Important | src/install/PackageManager/PackageManagerDirectories.rs:328-338 |
Fallback to node_modules/.cache removed for ALL cache-dir errors, not just the infinite-loop case |
| 🟡 Nit | src/install/PackageManager/PackageManagerDirectories.rs:331-333 |
Error message hand-quotes path instead of using bun_fmt::quote |
Annotations
Check failure on line 338 in src/install/PackageManager/PackageManagerDirectories.rs
claude / Claude Code Review
Fallback to node_modules/.cache removed for ALL cache-dir errors, not just the infinite-loop case
Removing the loop drops the `node_modules/.cache` fallback for **all** `make_open_path` errors, not just the dangling-symlink case — the old `Err(_) => { clear CACHE; continue }` was reachable for `EACCES`/`EROFS`/`ENOTDIR` on the implicitly-derived `$HOME/.bun/install/cache` path (root-owned `~/.bun` after a prior `sudo`, read-only $HOME in containers). Those users previously got a working per-project cache; now `bun install` hard-exits. With `make_path_with` fixed the infinite loop is already
Check warning on line 333 in src/install/PackageManager/PackageManagerDirectories.rs
claude / Claude Code Review
Error message hand-quotes path instead of using bun_fmt::quote
Nit: REVIEW.md ("Error messages are reviewed word-for-word as code") specifies "quoting via `bun.fmt.quote`", and every other path-quoting error site in `src/install/PackageManager/` uses `bun_fmt::quote(...)` rather than hand-rolled `\"{}\"` around `bun_fmt::s`. Consider `bun_fmt::quote(&cache_dir.path)` here — it also JSON-escapes embedded quotes/control chars/non-ASCII in the path, which the manual quotes do not. For the temp-dir path used in the test, `quote` emits the same `"..."` form, so
Loading