Claude / Claude Code Review
completed
Aug 5, 2026 in 29m 25s
Code review found 3 potential issues
Found 2 candidates, confirmed 3. See review comments for details.
Details
| Severity | Count |
|---|---|
| 🔴 Important | 0 |
| 🟡 Nit | 2 |
| 🟣 Pre-existing | 1 |
| Severity | File:Line | Issue |
|---|---|---|
| 🟡 Nit | src/install/PackageInstall.rs:1726-1750 |
install_with_copyfile (hardlink's direct EXDEV fallback) still Global::crash()s on the same rename-away race |
| 🟣 Pre-existing | src/install/PackageInstall.rs:1712-1725 |
install_with_symlink EEXIST retry passes wrong target, creating self-referential symlinks |
Annotations
Check warning on line 1750 in src/install/PackageInstall.rs
claude / Claude Code Review
install_with_copyfile (hardlink's direct EXDEV fallback) still Global::crash()s on the same rename-away race
🟡 Same-class sibling: `install_with_copyfile` — the direct EXDEV fallback from `install_with_hardlink` on this same dispatch (L2572→2611) — has the identical stale-dirfd race and still ends in `Global::crash()` (POSIX L1496-1534, Windows L1433-1473) instead of getting the reopen-from-`dest_dir` recovery added here. Pre-existing and narrower (needs cache and node_modules on different filesystems, or explicit `--backend copyfile`), so not blocking; consider giving copyfile the same ENOENT recovery
Check notice on line 1725 in src/install/PackageInstall.rs
claude / Claude Code Review
install_with_symlink EEXIST retry passes wrong target, creating self-referential symlinks
Pre-existing, not touched by this PR — surfacing under the same 'fix the whole class' rule as the Hardlinker.rs note: `install_with_symlink`'s POSIX EEXIST retry (PackageInstall.rs:1948) passes `entry.basename` as the symlink target instead of `target`, so the retry creates `<dest>/<entry.path>` → `<basename>`, which resolves to itself (ELOOP on read). Any `bun install --backend symlink` where a destination file already exists — the concurrent-install scenario this PR handles for hardlink — inst
Loading