install: retry cache publish renames on Windows while a scanner holds a file open - #35568
Open
robobun wants to merge 8 commits into
Open
install: retry cache publish renames on Windows while a scanner holds a file open#35568robobun wants to merge 8 commits into
robobun wants to merge 8 commits into
Claude / Claude Code Review
completed
Aug 13, 2026 in 19m 5s
Code review found 3 potential issues
Found 5 candidates, confirmed 3. See review comments for details.
Details
| Severity | Count |
|---|---|
| 🔴 Important | 0 |
| 🟡 Nit | 3 |
| 🟣 Pre-existing | 0 |
| Severity | File:Line | Issue |
|---|---|---|
| 🟡 Nit | src/install/cache_rename.rs:20 |
ENV_VAR_NAME string literal duplicates the canonical name in env_var.rs |
| 🟡 Nit | src/install/cache_rename.rs:20 |
PR description stale again after 9ee07e0: env var and test file were renamed |
| 🟡 Nit | src/install/isolated_install/Installer.rs:2484-2489 |
--force swap-out rename (final_ → old) is not covered by the RenameRetry budget |
Annotations
Check warning on line 20 in src/install/cache_rename.rs
claude / Claude Code Review
ENV_VAR_NAME string literal duplicates the canonical name in env_var.rs
`ENV_VAR_NAME` hard-codes `"BUN_INSTALL_WINDOWS_RENAME_RETRY_MS"` as a second string literal, but the env-var machinery already exposes the canonical name via `bun_core::env_var::BUN_INSTALL_WINDOWS_RENAME_RETRY_MS.key()` (see `HOME.key()` at create_command.rs:1812). This var was already renamed once during review (`WIN32_AV` → `WINDOWS_RENAME`, commit 127c5eb5); a future rename that misses this literal leaves the user-facing hint in `ExhaustedHint::fmt` naming a nonexistent variable. Secondaril
Check warning on line 20 in src/install/cache_rename.rs
claude / Claude Code Review
PR description stale again after 9ee07e05: env var and test file were renamed
The PR description is stale again after 9ee07e05: the **Fix** section names `BUN_INSTALL_WIN32_AV_RETRY_MS` but the env var is now `BUN_INSTALL_WINDOWS_RENAME_RETRY_MS` (env_var.rs:114, cache_rename.rs:20); the **Verification** section names `test/cli/install/bun-install-windows-locked-temp.test.ts` but the file is `bun-install-windows-rename-retry.test.ts`, describes a 500ms hold when `HOLD_MS = 2000`, and covers only the tarball rename when the fix now also covers `patch_install` and the isola
Check warning on line 2489 in src/install/isolated_install/Installer.rs
claude / Claude Code Review
--force swap-out rename (final_ → old) is not covered by the RenameRetry budget
Under `--force`, the swap-out rename `sys::renameat(final_ → old)` at lines 2484-2489 is the only rename in `commit_global_store_entry` not covered by the `RenameRetry` budget — the initial `staging→final_` (2451-2466) and the post-swap `staging→final_` (2490-2517) both are. If a scanner/Search Indexer holds a handle without `FILE_SHARE_DELETE` on a file inside the *existing* `final_` entry, this rename fails `STATUS_ACCESS_DENIED` for the same MS-FSA 2.1.5.14.11 reason and returns `Err(swap_err
Loading