install: retry cache publish renames on Windows while a scanner holds a file open

[robobun]

Fixes #11250

Problem

• bun install on Windows fails while publishing a package into the cache:

  error: moving "@oxlint/win32-x64" to cache dir failed
  EPERM: Operation not permitted (NtSetInformationFile())
  

  Reporters clear it by disabling their scanner (Sophos, Defender, JAMF) or by re-running until it sticks.
• bun publishes into the cache by renaming a directory it just wrote. NTFS fails a directory rename with STATUS_ACCESS_DENIED while any process holds a handle without FILE_SHARE_DELETE on a file inside it, and that is how scanners open freshly written files. bun's own handles request FILE_SHARE_DELETE; the blocking handle is the scanner's.
• Three publishes do this rename. The tarball one (extract_tarball.rs) retried, but for 4 attempts totalling 150ms of sleep, shorter than a scan of a multi-MB binary. The patched-package one (patch_install.rs, reported as renaming changes to cache dir: EPERM) did not retry. The global virtual store one (isolated_install/Installer.rs::commit_global_store_entry) treated any EPERM on Windows as "a concurrent install published first", deleted its own staging dir and returned success, so the install exited 0 with node_modules/.bun/<pkg> pointing at a directory that does not exist.

Fix

• New bun_install::cache_rename::RenameRetry: a deadline (BUN_INSTALL_WINDOWS_RENAME_RETRY_MS, default 5000) with graceful-fs's backoff (10ms more per attempt, capped at 100ms). is_transient is EPERM/EACCES/EBUSY on Windows and never true on POSIX, where open handles do not block renames and those errnos are real failures.
• All three publishes (and the --force swap-out rename in the global store path) loop on it. The tarball path keeps retrying EXIST/NOTEMPTY as before, since that arm also covers concurrent installs. is_rename_collision now only counts EPERM as a collision when the destination actually exists, which is what makes the global store path retry instead of silently discarding the package.
• When the budget runs out, the reported error says how long it waited and names the variable:

  error: moving "demo" to cache dir failed (gave up after retrying for 1086ms; usually another process such as antivirus has a file in the directory open. Set BUN_INSTALL_WINDOWS_RENAME_RETRY_MS to wait longer)
  EPERM: Operation not permitted (NtSetInformationFile())
  

• Why 5s: it outlasts a real-time scan of a multi-MB binary with margin (SQLite's default for the same retry is 1.4s and it exposes the knob because that is not always enough; graceful-fs, which npm and yarn go through, waits 60s). It is also what a permanent failure such as an unwritable cache dir now costs per package before being reported, which is why it is not 60s.
• Verified by test/cli/install/bun-install-windows-rename-retry.test.ts (Windows only; the changed behaviour is unreachable elsewhere and the file skips). It covers each of the three publishes twice against a fixture that opens an extracted file through CreateFileW without FILE_SHARE_DELETE: with the default budget the install must outlast a 2s hold, and with BUN_INSTALL_WINDOWS_RENAME_RETRY_MS=0 it must fail immediately and name the variable, which also shows the held handle is what blocks the rename. For the tarball case the registry withholds the second half of the tarball until the fixture reports it has the handle.
• cargo clippy -p bun_install -p bun_core and cargo check -p bun_install --target x86_64-pc-windows-msvc pass.

Background

• Cache publish: every package goes through extract or build into a private directory, then one rename to its final cache path, so readers only ever see complete entries. The three publishes above are the three places that rename happens.
• Global virtual store: with linker = "isolated" and BUN_INSTALL_GLOBAL_STORE=1, each package's node_modules tree is built in <cache>/links/<entry>.tmp-<pid> and renamed to <cache>/links/<entry>; a project's node_modules/.bun/<pkg> is a symlink to that entry. Two installs racing on the same entry is expected and the loser keeps the winner's copy, which is the collision logic this change narrows.
• FILE_SHARE_DELETE: the Windows share mode a process must opt into when opening a file for the file, or any directory above it, to be renamed or deleted while the handle is open. Scanners typically do not set it.

Test run on Windows Server 2019 against main (system bun) and against this branch (bun bd)

bun test v1.4.0-canary.1 (9a543cc18)
(fail) ... > global virtual store entry: default budget outlasts a 2s hold [450.00ms]
(fail) ... > global virtual store entry: BUN_INSTALL_WINDOWS_RENAME_RETRY_MS=0 fails at once and names the variable [484.27ms]
(fail) ... > extracted tarball: default budget outlasts a 2s hold [524.08ms]
(fail) ... > extracted tarball: BUN_INSTALL_WINDOWS_RENAME_RETRY_MS=0 fails at once and names the variable [606.25ms]
(fail) ... > patched package: default budget outlasts a 2s hold [603.13ms]
(fail) ... > patched package: BUN_INSTALL_WINDOWS_RENAME_RETRY_MS=0 fails at once and names the variable [600.00ms]
 0 pass
 6 fail

On main the tarball and patch default-budget cases fail with the errors quoted above; the global store case exits 0 and the existsSync on the entry's bin.exe is false; the three budget=0 cases fail because nothing names the variable.

bun test v1.4.0 (2172ab4f2)
(pass) ... > extracted tarball: BUN_INSTALL_WINDOWS_RENAME_RETRY_MS=0 fails at once and names the variable [1351.66ms]
(pass) ... > global virtual store entry: BUN_INSTALL_WINDOWS_RENAME_RETRY_MS=0 fails at once and names the variable [1334.46ms]
(pass) ... > patched package: BUN_INSTALL_WINDOWS_RENAME_RETRY_MS=0 fails at once and names the variable [1411.24ms]
(pass) ... > extracted tarball: default budget outlasts a 2s hold [3029.74ms]
(pass) ... > patched package: default budget outlasts a 2s hold [3206.85ms]
(pass) ... > global virtual store entry: default budget outlasts a 2s hold [3360.34ms]
 6 pass
 0 fail

Related: #33884 reworks the concurrent-install arm of the tarball retry loop; #33029 handles non-NTFS volumes, which fail with a different status.

---

no test proof · iteration 3 · Platform-specific test(s) that do not run on this machine. Deferring to CI, which covers all platforms: test/cli/install/bun-install-windows-rename-retry.test.ts

[coderabbitai]

Warning

Review limit reached

@robobun, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 9 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: b3d81cd4-2b5f-4202-a4d6-f1ef0ea7b94e

📥 Commits

Reviewing files that changed from the base of the PR and between a0f399f and 2172ab4.

📒 Files selected for processing (8)

• src/bun_core/env_var.rs
• src/install/cache_rename.rs
• src/install/extract_tarball.rs
• src/install/isolated_install/Installer.rs
• src/install/lib.rs
• src/install/patch_install.rs
• test/cli/install/bun-install-windows-rename-retry-fixture.ts
• test/cli/install/bun-install-windows-rename-retry.test.ts

---

_{Comment @coderabbitai help to get the list of available commands.}

[robobun]

^{Updated 11:43 PM PT - Aug 12th, 2026}

❌ @robobun, your commit 2172ab4 has some failures in Build #93849 (All Failures)

---

🧪   To try this PR locally:

bunx bun-pr 35568

That installs a local version of the PR into your bun-35568 executable, so you can run:

bun-35568 --bun

[claude]

Thanks for addressing the earlier nits (Bun.$ for tar, 15s comment). The Rust change itself is minimal — two constants in the existing Windows retry loop, adopting SQLite's winIoerrRetry schedule — and looks correct. Deferring to a human mainly for the new Windows-only test: it coordinates a stalling registry, an FFI-based handle-holder subprocess, and stdout signaling, which is intricate enough that a maintainer should confirm it lands green (and non-flaky) on Windows CI before merge.

The open comment-cop flag on extract_tarball.rs:557 looks like a false positive — the 4-line comment is the same length as the one it replaces and carries the mechanism + schedule citation, which is the durable content REVIEW.md asks for.

What was reviewed

• Retry-loop change: same errno set (PERM/BUSY/EXIST/NOTEMPTY), only MAX_RETRIES 4→10 and backoff 10<<(n-1) → 25*n; no overflow, error path unchanged.
• Test env knobs: confirmed BUN_INSTALL_STREAMING_MIN_SIZE and BUN_TMPDIR are read by src/; test asserts HELD so a pass can't be vacuous if the blocker misses the window.
• Cleanup: using/await using on tempdir, server, both subprocesses; held/ready are resolved on reader EOF so the server pull can't hang the teardown.

Extended reasoning...

Overview

The PR extends the Windows-only retry budget in src/install/extract_tarball.rs when renaming a freshly extracted temp directory into the package cache. It changes MAX_RETRIES from 4 to 10 and the per-retry sleep from exponential 10 << (n-1) ms (~150ms total) to linear 25 * n ms (~1375ms total), matching SQLite's winIoerrRetry. The retried errno set, the destination-swap fallback, and the final error message are all unchanged. It adds a Windows-only regression test (test/regression/issue/11250.test.ts) plus an FFI fixture (test/cli/install/bun-install-windows-locked-temp-fixture.ts) that opens the extracted file via CreateFileW without FILE_SHARE_DELETE to deterministically reproduce the AV-scanner race.

Security risks

None identified. No new inputs are parsed; the change only lengthens an existing bounded retry-and-sleep loop inside #[cfg(windows)]. The test uses a local Bun.serve({ port: 0 }) registry and a temp cache/tmp dir — no external network, no privilege changes.

Level of scrutiny

The production diff is ~10 lines of constant/comment changes in an already-retrying loop and is low-risk in isolation. The test, however, is ~275 lines across two new files and coordinates three moving parts (a mid-response-stalling HTTP server, a busy-polling FFI subprocess that grabs a Win32 handle, and bun install itself) via stdout token parsing. That's the kind of Windows-CI-only harness where flakiness only shows up on the actual runners, so it warrants a maintainer eye on the first CI run rather than a bot approval.

Other factors

• My two earlier nits (use Bun.$ instead of spawnSync for tar; fix the 30s→15s doc mismatch in the fixture header) were both addressed in commit 0d7eee8 and the threads are resolved.
• I verified the env vars the test relies on are real: BUN_INSTALL_STREAMING_MIN_SIZE is defined in src/bun_core/env_var.rs:97 and consumed in src/install/TarballStream.rs:179; BUN_TMPDIR is read in PackageManagerDirectories.rs. The test asserts blockerOut contains HELD, so if the fixture fails to catch the extraction window the test fails rather than vacuously passing.
• Cleanup paths look sound: using tempDir, await using server, await using on both subprocesses, and the background stdout reader resolves both ready and held on EOF so the server's stalled pull() can't wedge teardown.
• The unresolved comment-cop inline on extract_tarball.rs:557 is a mechanical multi-line-comment lint on claude PRs; the new comment is no longer than what it replaces and documents the NTFS invariant plus the SQLite schedule source, which is exactly the "durable non-obvious content" REVIEW.md calls for. A human can dismiss it, but I didn't want to approve over an open bot flag.

[claude]

No issues found in this pass. The retry-loop change itself is small and the earlier nits are all addressed, but I'd like a human to sign off on two things: (1) the new user-facing env var BUN_INSTALL_WIN32_AV_RETRY_MS and its 5-second default — a genuine EPERM now surfaces after ~5s instead of ~150ms, which is a UX/design call; and (2) the Windows-only test, which is elaborate (FFI to CreateFileW, a stalling registry, streaming coordination) and only exercised on Windows CI — build #80760 was still running at review time.

What was reviewed:

• .get().unwrap() on the new env var is safe — it has default: 5_000, so get() is always Some.
• Retry errno set (NOTEMPTY/PERM/BUSY/EXIST) and the error path are unchanged; only the loop bound moved from a 4-count to start.elapsed() < budget.
• On the AV path the per-iteration renameat(cache_dir, folder_name, …) targets a non-existent destination, so ~50 retries don't repeatedly delete_tree.
• Test requires HELD in the blocker output, so a pass where the fixture missed the extraction window fails rather than silently passing.

Extended reasoning...

Overview

The PR extends the Windows-only retry loop in src/install/extract_tarball.rs that renames a freshly extracted temp directory into the package cache. It swaps a fixed 4-attempt exponential backoff (~150ms total) for a deadline-based loop: linear backoff of 10ms increments capped at 100ms, retried until BUN_INSTALL_WIN32_AV_RETRY_MS (new env var, default 5000ms) elapses. A one-line env-var declaration is added to src/bun_core/env_var.rs. Two new test files add a Windows-only regression test that simulates an AV scanner holding an extracted file open without FILE_SHARE_DELETE via bun:ffi → kernel32!CreateFileW, with a local registry that stalls mid-tarball until the handle is held.

Security risks

None identified. The change is confined to #[cfg(windows)] install-cache rename retry logic; no new inputs are parsed, no auth/crypto/path-validation is touched. The env var goes through the existing new!(unsigned) machinery which parses via bun_core::fmt::parse_int and falls back to the default on parse failure.

Level of scrutiny

Medium. The production-code delta is small (~15 lines) and mechanical — the errno match arms, error message, and cleanup path are unchanged; only the loop bound changed from a counter to Instant::elapsed() < budget. However, it introduces a new user-facing env var (API surface) and picks a default that trades diagnostic latency (genuine EPERM now takes ~5s to surface) for AV robustness. That default is a design call the PR justifies well (between SQLite's 1.375s and graceful-fs's 60s), but a maintainer should confirm it. The test is intricate — FFI, a stalling ReadableStream registry, subprocess stdout coordination — and only runs on Windows CI, which hadn't reported at review time.

Other factors

All three of my prior nits (use Bun.$ for tar, fixture header 30s→15s, PR description out of sync) are resolved and marked as such; the description now matches the shipped code. The bug-hunting system found nothing this run. The comment-cop bot flagged the long inline comment twice; it's now collapsed to a single long line, which satisfies the rule if not readability. The test asserts blocker: expect.stringContaining("HELD") in the combined-object assertion, so a run where the fixture failed to grab the handle in time would fail the test rather than vacuously pass — good. BUN_INSTALL_STREAMING_MIN_SIZE: "1" forces the streaming extractor so bin.exe is on disk before the registry stall; the fixture's tryOpenNoShareDelete failing on directories (it will hit package.json or bin.exe) is handled by looping over inner until a handle succeeds. Given the new env var and the pending Windows CI run, I'm deferring rather than approving.

[robobun]

CI on build #80760: the diff is green. All Windows 11 aarch64 test shards passed (including the new bun-install-windows-locked-temp.test.ts). Windows x64 build expired/timed out before tests could run (infra, unrelated to this change). The two flagged tests on alpine aarch64 (webview-chrome, bun-security-scanner-matrix-with-node-modules) both passed on retry and are unrelated.

The previous build #80391 hit the same "step failed outside runner" build coordination issue across most lanes, which ci:errors reports as pre-existing on main.

Ready for a maintainer to look at; the Windows x64 lane may need a manual retry.

[alii]

Requesting changes.

• Merged as-is the new test file fails on every platform: it imports stderrForInstall, which main removed in #37000.
• The retry covers the tarball cache move only. patchedDependencies renames into the cache through the same call with no retry, and the env var does not reach it.
• The test needs a case that shows the held handle blocks the rename at all and exercises BUN_INSTALL_WIN32_AV_RETRY_MS.
• Smaller: nothing says where 5s comes from, and the error a user sees after the budget runs out does not name the knob.

[robobun]

@alii reworked per your review, at 2172ab4:

• Rebased; stderrForInstall is gone.
• The retry now lives in cache_rename::RenameRetry and wraps all three cache publishes (tarball, patch_install, commit_global_store_entry including the --force swap-out). is_rename_collision only counts EPERM as a collision when the destination exists, which fixes the global store path silently discarding the package.
• Variable renamed to BUN_INSTALL_WINDOWS_RENAME_RETRY_MS; its declaration explains the 5s; the error after the budget is spent reports how long it waited and names the variable.
• Test now covers each publish with a 2s hold against the default budget and with the budget set to 0, asserting failure and the variable name. Runs against main and this branch on the Windows box are in the description (0/6 vs 6/6).

Description rewritten to match.

[alii]

Looks ready to merge from this side. Checked 2172ab4 against main: the stale harness import is gone, RenameRetry wraps the tarball move, patch_install's renameat_concurrently and both renames plus the --force swap-out in commit_global_store_entry, is_rename_collision no longer turns a held staging dir into a silent success, the budget default and the exhausted error name the variable, and the rewritten test has a budget=0 case per publish that fails on main. The three publishes are the only fatal cache renames (the manifest cache rename in npm.rs is best-effort). Nothing on main touched these files since the merge base and it merges clean. The Windows runs and the windows-msvc cargo check in the description are not reproduced here.