Conversation
|
Warning Review limit reached
Next review available in: 3 seconds Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the 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 configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (12)
WalkthroughChangesWindows BunX shim metadata now uses an executable Windows BunX metadata
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Comment |
|
Built and verified on Windows x64: all 47 Review rounds addressed in 8e30496 / 6bc33cd / 21566d3 (SHAs changed after rebase): stale-stream unlink before sidecar fallback, bounds+u16 guards on both probe paths, keep the sidecar when the exe rewrite hits EBUSY, CI: diff is green. Buildkite #83970 and #84306 both ran the 47 shim tests and all modified test files cleanly; the red lanes are unrelated flakes ( |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/bun_core/feature_flags.rs`:
- Around line 100-102: Update the Windows fast-path documentation comment in
feature flags to reference the actual entry point `try_startup_from_bun_js`
instead of the stale `bun_shim_impl.tryStartupFromBunJS` spelling, preserving
the rest of the explanation.
In `@src/install/bin.rs`:
- Around line 1238-1289: Before writing the `.bunx` sidecar in
`write_file_os_path`, unlink the existing `abs_ads_file` stream so fallback
cannot leave stale ADS metadata; perform this while its path remains valid,
before rewriting `dest_buf`, and ignore the unlink result. Consolidate the
duplicated `.bunx` suffix/path construction into one helper or closure reused by
the ADS-success cleanup and fallback paths.
In `@src/install/windows-shim/bun_shim_impl.rs`:
- Around line 656-690: Guard both Windows probe paths against buffer overflow:
in src/install/windows-shim/bun_shim_impl.rs lines 656-690, validate the
required “:bunx” layout fits within BUF1_LEN before either write_unaligned call,
then return InvalidShimBounds or proceed directly to the sidecar attempt on
overflow; in src/runtime/cli/run_command.rs lines 4044-4048, check that ads_len
is less than direct_launch_buffer.len() before the stream-probe slice/write and
fall back to the sidecar path when it does not fit.
In `@src/runtime/cli/run_command.rs`:
- Around line 4044-4048: Guard the `.exe:bunx` probe in the run-fast-path logic
around `ads_len` and `direct_launch_buffer`: compute whether the additional
suffix fits within the `WPathBuffer` capacity before writing or logging the
expanded slice. When it does not fit, skip the stream-open/launcher probe and
proceed directly to the sidecar probe; only attempt the stream open when
`ads_fits` is true.
In `@test/cli/install/bun-install-registry.test.ts`:
- Around line 8797-8801: Update the subprocess assertions around stdout, stderr,
and exited so all three promises are consumed concurrently rather than awaiting
stderr before stdout. Preserve the existing trimmed-output and exit-code
expectations while using concurrent promise handling for the process pipes.
In `@test/cli/install/bunx.test.ts`:
- Around line 1215-1216: Remove the layout-explanation comment in
test/cli/install/bunx.test.ts at lines 1215-1216; the ADS-first fallback is
clear from the test code. Also remove the fast-path narration in
test/regression/issue/13316.test.ts at line 42, leaving only the required issue
URL comment for the regression test.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 62d277b6-e7f2-41f2-aac0-6d952aa2b7d6
📒 Files selected for processing (12)
src/bun_core/feature_flags.rssrc/install/bin.rssrc/install/windows-shim/bun_shim_impl.rssrc/runtime/cli/run_command.rstest/cli/install/bun-install-lifecycle-scripts.test.tstest/cli/install/bun-install-registry.test.tstest/cli/install/bun-publish.test.tstest/cli/install/bun-run.test.tstest/cli/install/bunx.test.tstest/cli/install/isolated-install.test.tstest/harness.tstest/regression/issue/13316.test.ts
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/install/bin.rs (1)
1203-1236: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winKeep
.bunxwhen the exe rewrite is skippedIf the
.exewrite hitsEBUSY, the old shim stays on disk. Deleting the sidecar here can leave that binary without metadata, so only unlink.bunxafter a successful exe rewrite.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/install/bin.rs` around lines 1203 - 1236, Update the executable rewrite flow around sys::File::write_file_os_path so the .bunx sidecar is unlinked only after the write returns Ok(()). Preserve the existing EBUSY branch without deleting .bunx, since the old shim remains in place; ensure other successful rewrite paths still perform the sidecar cleanup.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/install/windows-shim/bun_shim_impl.rs`:
- Around line 659-702: The ADS and sidecar length checks in the metadata-opening
flow must also ensure the byte length fits in u16 before conversion. Update the
guards around ads_path_len and sidecar_path_len to reject any length where 2 *
length exceeds u16::MAX, returning LauncherMode::fail(MODE,
FailReason::InvalidShimBounds) before open_metadata; remove reliance on the
fallible conversion succeeding.
---
Outside diff comments:
In `@src/install/bin.rs`:
- Around line 1203-1236: Update the executable rewrite flow around
sys::File::write_file_os_path so the .bunx sidecar is unlinked only after the
write returns Ok(()). Preserve the existing EBUSY branch without deleting .bunx,
since the old shim remains in place; ensure other successful rewrite paths still
perform the sidecar cleanup.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: f1090797-a752-4d67-9707-20d23e697795
📒 Files selected for processing (7)
src/bun_core/feature_flags.rssrc/install/bin.rssrc/install/windows-shim/bun_shim_impl.rssrc/runtime/cli/run_command.rstest/cli/install/bun-install-registry.test.tstest/cli/install/bunx.test.tstest/regression/issue/13316.test.ts
💤 Files with no reviewable changes (2)
- test/regression/issue/13316.test.ts
- test/cli/install/bunx.test.ts
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/install/bin.rs (1)
1198-1243: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winAdd a wide-path capacity guard before suffix writes
src/install/bin.rs:1198-1243
WPathBufferis only 32767u16s, and.exe:bunxneeds 10 units including the NUL. The byte-length checks above don’t leave UTF-16 headroom here, so a longabs_destcan make this slice write panic. Guard once onabs_dest_w_len + 10 > dest_buf.len()(covers.exe,.exe:bunx, and.bunx) or use a larger buffer.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/install/bin.rs` around lines 1198 - 1243, Add a capacity guard before the suffix writes in the executable path flow, checking whether abs_dest_w_len + 10 exceeds dest_buf.len() and returning through the existing error path when it does. Ensure this single guard protects the .exe, .exe:bunx, and .bunx suffix constructions before any slice copy occurs.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@src/install/bin.rs`:
- Around line 1198-1243: Add a capacity guard before the suffix writes in the
executable path flow, checking whether abs_dest_w_len + 10 exceeds
dest_buf.len() and returning through the existing error path when it does.
Ensure this single guard protects the .exe, .exe:bunx, and .bunx suffix
constructions before any slice copy occurs.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 2dbd06e6-d924-4378-9405-67c2cd9cf34b
📒 Files selected for processing (4)
src/install/bin.rssrc/install/windows-shim/bun_shim_impl.rstest/cli/install/bun-install-lifecycle-scripts.test.tstest/cli/install/bun-install-registry.test.ts
There was a problem hiding this comment.
My earlier three findings (the what_bin_bins ReferenceError, the unasserted first-install exit code, and the stale-ADS-on-sidecar-fallback asymmetry) are all addressed in the latest commits, and I didn't find anything new. That said, this changes the on-disk Windows bin-shim layout and reworks a fair amount of unsafe buffer/offset arithmetic in the freestanding shim plus the BunXFastPath reader — worth a human pass on the design and the pointer math.
What was reviewed:
create_windows_shimwrite ordering (exe → ADS → sidecar), EBUSY/ENOENT branches, and stale-file cleanup in both directionsbun_shim_impl.rsADS/sidecar probe: buffer-length guards,nt_len_bytesu16 bound, and that theread_ptrnode_modules walk is unchanged by the new suffixrun_command.rs:.bunxtail is restored and NUL-terminated beforesplit_at_muton both the ADS-hit and fallback paths- Test/harness updates accept both layouts;
what_bin_binsfix verified at all four call sites
Extended reasoning...
Overview
This PR moves Windows node_modules/.bin shim metadata from a <name>.bunx sidecar file into a :bunx NTFS alternate data stream on <name>.exe, with the sidecar retained as a fallback for volumes without named streams. It touches the writer (src/install/bin.rs create_windows_shim), both readers (the standalone bun_shim_impl.rs launcher and BunXFastPath::try_launch in run_command.rs), a doc comment in feature_flags.rs, the toHaveBins/toBeValidBin harness matchers, and seven test files.
Security risks
None identified. The metadata content and encoding are unchanged; only the storage location moves. Paths are still derived from the process's own image path (standalone) or a caller-built buffer (fast path), not from untrusted input. New buffer writes are bounds-guarded (nt_len_bytes, ads_len < direct_launch_buffer.len(), the pre-copy BUF1_LEN check).
Level of scrutiny
High. This is a design change to the Windows binary-launch path, with:
- ~100 lines of new/rewritten unsafe raw-pointer arithmetic in a
no_stdfreestanding PE (bun_shim_impl.rs) where a miscomputed offset is UB, not a panic. - Cross-version compatibility handling: the EBUSY branch keeps a pre-ADS shim exe on disk and writes both the ADS and a fresh sidecar so the old PE can still find metadata; that interaction is subtle and Windows-only.
- Filesystem-dependent fallback (NTFS vs exFAT/SMB) that CI can't easily exercise.
None of that is a bug I can point to — it reads correctly to me — but it's exactly the class of change REVIEW.md flags for careful human review of memory-safety and layering.
Other factors
All three of my prior inline findings are fixed and verified in the current diff:
what_bin_binsis re-declared with afilterBunxhelper applied at all four readdir sites (bun-install-lifecycle-scripts.test.ts:1118-1244).- The first install in the stale-sidecar test now asserts
expect(await spawn(...).exited).toBe(0). - The sidecar-fallback arm in
bin.rsnow best-effort-unlinks the ADS before writing the sidecar.
CodeRabbit's bounds/u16 concerns were also addressed (the nt_len_bytes helper and ads_len < direct_launch_buffer.len() guard). The three new Windows tests are in the existing suite and cover the ADS layout, sidecar fallback, and stale-sidecar cleanup on reinstall. Test-only changes to other files are mechanical (.bunx → .exe).
Given the scope — an on-disk format change plus non-trivial unsafe code in the Windows launch path — this should get a human reviewer's sign-off rather than bot approval.
|
Updated 3:37 PM PT - Jul 28th, 2026
❌ @robobun, your commit 1ae0917 has some failures in 🧪 To try this PR locally: bunx bun-pr 36200That installs a local version of the PR into your bun-36200 --bun |
…sidecar On Windows, node_modules/.bin previously held two files per bin: <name>.exe (the embedded bun_shim_impl stub) and <name>.bunx (the per-bin metadata: relative target path + parsed shebang). The metadata now lives in a :bunx alternate data stream on the exe itself, so .bin holds one file per bin and the metadata cannot orphan or desync from its stub. The two-file layout remains the fallback when the stream cannot be created (exFAT/FAT32 and some network shares reject named streams). The shim stub, the bun.exe fast path, and the installer all probe the stream first and fall back to the sidecar: - bin.rs create_windows_shim: write the exe first, then try <name>.exe:bunx (deleting any stale .bunx sidecar on success); on any error write <name>.bunx instead. - bun_shim_impl.rs (standalone): NtCreateFile <image>.exe:bunx, fall back to <image>.bunx on failure. The buf1 path-walk that derives node_modules is driven by image_path_b_len and is unchanged by either suffix. - run_command.rs BunXFastPath: probe <name>.exe:bunx before <name>.bunx; base_path handed to the launcher stays .bunx-shaped. - unlink_bin_or_shim: unchanged (already deletes both .exe and .bunx; deleting the exe removes its streams). Tests: toBeValidBin/toHaveBins accept either layout; new Windows tests cover the one-file .bin layout, the sidecar fallback in the standalone shim, and stale-.bunx cleanup on reinstall.
…be paths - bin.rs: unlink any pre-existing :bunx stream before writing the sidecar fallback so a transient ADS open failure cannot leave stale metadata that wins the launcher's probe. Fold the duplicated .bunx path construction. - bun_shim_impl.rs: guard the image-path copy and both suffix writes against BUF1_LEN; skip the stream probe (fall through to sidecar) when :bunx would not fit, fail InvalidShimBounds when the sidecar would not fit either. - run_command.rs: skip the stream probe when ads_len would exceed the WPathBuffer instead of panicking on the slice write. - feature_flags.rs: fix stale tryStartupFromBunJS reference. - tests: drain stdout/stderr/exited concurrently; drop narrative comments.
- bin.rs: when the exe rewrite hits EBUSY the on-disk PE may predate the stream probe, so write a fresh sidecar alongside the stream instead of deleting it. The sidecar is only removed when both the exe and the stream were written. - bun_shim_impl.rs: BUF1_LEN (32770) exceeds u16::MAX/2, so fold the UNICODE_STRING.Length conversion into the bounds guard and skip/fail on overflow rather than unwrap.
bun-install-lifecycle-scripts.test.ts: the earlier change removed the what_bin_bins declaration but three later assertions in the same test still referenced it. Restore the declaration (Windows branch now ["what-bin.exe"]) and filter .bunx entries at each readdir so either layout passes. bun-install-registry.test.ts: assert exit 0 on the first install in the stale-sidecar-cleanup test so a setup failure points at the right step.
66debb8 to
1ae0917
Compare
There was a problem hiding this comment.
The three findings from the earlier pass are all addressed and I didn't spot anything new, but this reworks the Windows bin-linking on-disk layout (ADS vs. sidecar) with unsafe buffer arithmetic in the shim and cross-version compat implications, so it's worth a human look before landing.
What was reviewed:
create_windows_shimreordering: exe-first then ADS, ENOENT mkdir retry moved to the exe write, EBUSY keeps the sidecar so a pre-ADS shim PE still finds metadata, and the fallback now best-effort-unlinks a stale stream.bun_shim_impl.rsstandalone probe: verified thent_len_bytesguard covers both the buf1 bound and theUNICODE_STRING.Lengthu16, and that the sidecar overwrite offset lands on the '.' (net +1 u16, within the earlier copy).BunXFastPath::try_launch: the.bunxtail is restored beforesplit_at_mut(path_len)so the launcher's downstream buffer math is unchanged; theads_len < buffer.len()guard degrades to sidecar instead of panicking.- Test/harness updates (
toHaveBins,what_bin_bins,.bunx→.exeexistence checks) accept both layouts.
Extended reasoning...
Overview
This PR changes the Windows node_modules/.bin layout from two files per bin (<name>.exe + <name>.bunx) to one (<name>.exe with a :bunx NTFS alternate data stream), keeping the sidecar as a fallback for volumes without named streams. It touches four native files — the bin-linker write path (bin.rs), the standalone shim's metadata open (bun_shim_impl.rs, unsafe pointer-arithmetic no_std code), the in-process fast path (run_command.rs), and a doc-only change to feature_flags.rs — plus eight test files including the shared toHaveBins/toBeValidBin matchers in harness.ts.
Security risks
None identified. The metadata contents and their consumers are unchanged; only the storage location moves. The unsafe writes in the shim now have explicit bounds guards (an improvement over the prior code, which had none for the +1-u16 case). No new user-controlled input reaches a syscall.
Level of scrutiny
High. This is a design decision (ADS as the primary store) with real tradeoffs a maintainer should weigh: ADS are silently dropped by many copy tools, some archive/backup software, and cross-filesystem moves; some AV/EDR flags them; and the fallback/probe ordering creates a compat matrix between old-bun-written .bin dirs and new-bun readers (and vice versa). The bun_shim_impl.rs change is in a hand-optimized no_std PE with raw pointer arithmetic, and the write path in bin.rs reorders the exe/metadata sequencing and moves the ENOENT-mkdir retry from the metadata open to the exe write. These are all correctness-critical for every bun install on Windows.
Other factors
All prior review feedback (CodeRabbit's buffer-bounds and u16-overflow guards, the earlier bug-hunt pass's what_bin_bins ReferenceError, first-install exit-code assertion, and stale-stream-on-fallback unlink) has been addressed in follow-up commits and the current diff reflects those fixes. The new tests cover the ADS layout, sidecar fallback, and stale-sidecar cleanup on reinstall, but they only run on Windows CI. Given the scope, the design choice, and the unsafe code involved, this should get a human sign-off rather than a bot approval.
…ll_jsc, node-fallbacks, and misc crates (#39420) Net -4,402 lines (172 files, +509 / -4,911). Everything removed has zero references across `src/`, `scripts/`, `test/`, `packages/` and the regenerated `build/debug/codegen/` output, and the removal builds: removing a Rust or C++ definition that still had a caller fails to compile or link, so a green build is the reference check for those; JS, codegen and manifest removals were additionally grepped by name (including `bun:internal-for-testing` consumers under `test/`). Most of these were first identified by the sweeps that were closed yesterday for merge conflicts (#37272, #37062, #38439, #37089, #38703). This PR re-applies the subset that still applies on current main, minus anything an open PR already deletes and minus small hunks in files that change daily (see "Left out" below), plus a few new finds. ### react_compiler (-1,166) - `validate_no_derived_computations_in_effects_exp` and its ~22 exclusive helpers/types (~1.1k lines in `validation/validate_no_derived_computations_in_effects.rs`). The pipeline only ever calls the non-`_exp` validation; the `_exp` env-config flag was parsed from fixture pragmas and read by nothing. `react-compiler-fixtures.test.ts` now lists the two pragmas as ignored instead of handled; the fixture suite still passes. - `SymbolHost` (back-compat alias of `Host`; `DESIGN.md` updated), `HirBox`, `is_use_state_type`, `default_true`. ### C++ bindings (-1,350 across 64 files) - `JSDOMConvertBufferSource.h`: the IDL typed-array specializations and `toPossiblyShared*Array` helpers for every element type no binding converts (only the Uint8Array/ArrayBuffer views are used). - `JSDOMPromiseDeferred.h/.cpp`: `resolveWithJSValue`, `resolveWithNewlyCreated`, `resolveCallbackValueWithNewlyCreated` and friends. - `IDLTypes.h`: the `IDLUnsupportedType` family, the IDB / WebGL / `ScheduledAction` wrappers and stale forward declarations; `JSDOMConvertDate.h/.cpp` (the only `IDLDate` converter) deleted along with its two includes. The 8-line `IDLDate` struct itself stays for now (see "Left out"). `JSDOMConvertScheduledAction.h`, orphaned in the same way, is already deleted by #35775, so it is not touched here. - `NetworkLoadMetrics.h`: WebKit networking-stack fields/accessors bun never reads. - Deleted files: `JSWorkerOptions.h/.cpp` (`JSWorker.cpp` builds `WorkerOptions` by hand), `JSMIMEBindings.h/.cpp` (`createMIMEBinding` had no callers; include dropped from `ZigGlobalObject.cpp`), `JSDOMIterator.cpp` (`addValueIterableMethods`). - `ncrypto.h/.cpp`: `peekError`, `BignumPointer::isZero`, `EVPKeyCtxPointer::sign`, unused copy/move `operator=` overloads and an `AsymmetricKeyEncodingConfig` constructor. - Smaller: `JSDOMOperationReturningPromise.h` (`call*ReturningOwnPromise`), `JSDOMAttribute.h` (`setPassingPropertyName`, `setStatic`), `JSDOMGuardedObject` (`DoNotRegisterWithGlobalObjectTag`), `Event`/`EventTarget` (`resetBeforeDispatch`, default-handled flags, `isNode()`), `EventEmitter::{eventTypes,eventListeners}`, `HTTPHeaderIdentifiers::identifierFor`, `JSURLPatternResult` `convertDictionary` stubs, `PerformanceTiming::monotonicTimeToIntegerMilliseconds`, `ContextDestructionObserver::protectedScriptExecutionContext`, `expectedEnumerationValues<CryptoKeyUsage>`, `BufferSource::mutableData`/`toBufferSource`, `BunString__toInt32`, `BunString::utf8ByteLength`, the `Ref<StringImpl>` overload of `toCrossThreadShareable`, `normalWorld()`, `TextEncoding(const String&)`, `JSBuffer` `createBuffer`/`constructFromEncoding` overloads, the `JSX509Certificate` `m_infoAccess` lazy property and the non-legacy branch of `computeInfoAccess` (the prototype getter reads the view directly; x509 tests pass), `BakeAdditionsToGlobalObject::wrapComponent`, `jsFunction_lsanDoLeakCheck`, and the dead `BunObject+exports.h` macro entries (together with the Rust `BunObject_callback_nanoseconds` export the `nanoseconds` entry declared; `Bun.nanoseconds` is `functionBunNanoseconds` in `BunObject.cpp`). ### install_jsc / install_types / bun:internal-for-testing (-344) - `install_jsc/dependency_jsc.rs` and `update_request_jsc.rs` deleted: their only consumers were the `npa` / `npmTag` exports of `bun:internal-for-testing`, which no test imports. The `dispatch_js2native.rs` re-exports, the `generate-js2native.ts` file-map entry and the `lsanDoLeakCheck` export (tests use `isASANEnabled`) go with them. - `install_types/lib.rs`: the `ExternalString` / `SlicedString` / `SemverString` re-export modules; nothing names those paths (everything imports the types from `bun_semver`). New in this PR. ### node-fallbacks (-400) and codegen (-392) - `util.js`: a ~270 line commented-out `util.types` block. - `package.json` / `bun.lock` / `tsconfig.json`: dependencies and path mappings nothing imports (`esbuild`, `buffer`, `events`, `util`, `url`, `process`, `path-browserify`, `os-browserify`, `timers-browserify`, `tty-browserify`, `vm-browserify`, ...). `build-fallbacks.ts` marks every builtin name external, and the remaining sources only import the packages still listed; `bun install --frozen-lockfile` in the directory is a no-op and `bundler_browser.test.ts` passes. - `src/codegen/generate-unified-source-bundles.rb`: WebKit's Ruby generator, superseded by `scripts/build/unified.ts`; nothing invokes it. ### Rust crates (-1,200 across bun_jsc, bun_runtime, css, uws_sys and leaf crates) - bun_jsc: `BuiltinName::get` + `BUILTIN_NAME_MAP`, `MarkedArrayBuffer::to_js` (the `ArrayBuffer::alloc` Uint8Array arm and its `Bun__allocUint8ArrayForCopy` binding are kept, so `alloc` keeps mirroring `create`), the `__dangerouslySetPtr` wrapper `js_class_module!` emitted into every class, `ErrorBuilder::new`, `TopExceptionScope::new`, `Task::new`, `JSCell::to_js`, `JSGlobalObject::{to_js,ref_,ctx}`, `job::{on_js_thread,off_thread}`, `AbortReason` impl, `JSPromise` settle helpers, `UUID::ZERO`, `TagPayload::get`. - bun_runtime: the `target_os = "wasi"` directory-iterator backend in `dir_iterator.rs` (no shipped target is wasi and the `bun_sys::wasi` module it imports does not exist), the `Display` impls in `assert/myers_diff.rs`, the non-unix stub and not-macos escapes in `fs_events.rs` (the file is only compiled on macOS), `ArrayBufferSink::to_js`, unused re-exports in `node.rs` / `api/bun/spawn.rs` / `ffi/mod.rs` (with `abi_type` formatters narrowed to `pub(crate)`), `Error::UnableToDecode`, `MyersDiff::Error::OutOfMemory`. The first three are new in this PR. - css: the inherent `eql` / `to_css` / `parse` forwarders whose callers all go through the `CssEql` / `ToCss` / `Parse` trait impls (`values/calc.rs` and friends), `generics::{implement_eql,parse}`, `TokenList::parse_with_options`, `CssString::parse`. - uws_sys: the `uws_loop_defer`, `uws_res_clear_corked_socket`, `uws_ws_iterate_topics` and `uws_h3_req_get_parameter` C shims plus their Rust declarations and wrappers (`Loop::{uncork,wake,next_tick,run}`), `AnyResponse::init`, `SocketGroup::is_empty`, `socket.rs` `group()` accessors and the `SocketTcp`/`SocketTls` aliases, `Opcode::Close`, `WindowsLoop`. - leaf crates: `windows_sys` constants and their `bun_sys::windows` re-exports, `zlib`/`zlib_sys` declarations (`deflateInit_`, `inflateInit_`, the `gz*` file API, legacy type aliases), `sha_hmac` deprecated-API hashers (`SHA512` raw, `RIPEMD160`, `MD5_SHA1`, `Blake2` evp), `wyhash` `HashInt` impls for u16/u64, `libarchive` commented-out Zig-era callbacks, `bun_alloc` (`AllocError::name`, `usable_size`, `BSSList::init`), `clap::Error::WriteFailed`, `csrf` error variants, `pe::Error::{InputIsSigned,InsufficientSpace}`, `md` `Setextheader`, `opaque_mut_nn`, `cares_sys` `AddrInfo_hints::is_empty`, `boringssl_sys` constants, `errno` `Mode` re-exports, `bounded_array::get`, `string::write::Result`, `SplitIterator::rest`, `OutOfRangeValue` impls, `symbol::Map::init`, `sql_jsc` re-exports. ### src/js (-32) - Unused REPL primordials entries in `internal/repl/node-primordials.js`; with that gone `SafeWeakSet` had no importer, so `internal/primordials.js` stops exporting it (new in this PR). Unused export-object entries in `internal/fs/watch.ts` and `internal/readline/interface.js`. ### scripts (-9) - `glob-sources.ts` `src/*.c` pattern (matched nothing since `asan-config.c` was deleted), the write-only `BUN_DEP_*` defines in `depVersionsHeader.ts`, the unread `kqueue` config field. ### Verification - `bun bd` (full debug build) passes. - `bun run rust:check-all`: all 11 target triples ok (covers the windows/darwin-only removals in `windows_sys`, `sys/windows`, `zlib_sys/win32.rs`, `fs_events.rs`, `windows-shim`). - `cargo fmt --check`, `cargo clippy --workspace`, clang-format on every touched C++ file, prettier, and `bun run lint` are clean. - All of `test/internal/source-lints/` (including the new `dead-symbols-react-compiler-webcore-idl-misc.test.ts` that guards these symbols, and `dead-code-escapes` against the updated `dead-code-escape-limits.json`), `react-compiler-fixtures.test.ts`, `bundler_browser.test.ts`, css, cryptohasher/hash, node:assert, zlib, url, events, websocket, inspect and x509 tests pass. `serve.test.ts` has the same 4 failures as the unmodified release build in this container (IPv6 / root port range), nothing else. ### Left out on purpose (follow-up candidates) - The pre-engine inbound path in `h2_frame_parser.rs` (~2.2k lines, still dead, #37272's diff still applies cleanly): the file has had 15 commits in the last two weeks, so it is better landed on its own. - Deletions already owned by open PRs: simdutf wrappers (#38958), `getStackTraceForThrownValue` (#37450), `validateOneOf` (#38401), the redis error variants (#34829), `URL::from_js` (#33889 / #34577), `schema::api` re-exports (#37095), `FsPath` (#39327), `NodeJSFS` `Null` impl (#38065), the deprecated selector `to_css` (#33332). - Small hunks in high-churn files (`bindings.cpp`, `ZigGlobalObject.cpp`, `Blob.rs`, `streams.rs`, `BunProcess.cpp`, the `ManifestLoad::LoadFromMemory` parameter across `src/install`, the watcher `loader` parameter), and the `#[no_mangle]` statics (`Zig_ErrorCode*`, `Bun__versions_*`) nothing on the C++ side reads. - `VM::has_termination_request` and its `JSC__VM__hasTerminationRequest` shim in `bindings.cpp`: a dead pair, kept intact here because `bindings.cpp` is the most actively edited file in the tree; both halves go together in a follow-up. - `IDLDate` in `IDLTypes.h`: its only converter is deleted here, but the struct itself is left in place so this PR's file deletions stay independent of the header edits; removing the struct is a one-hunk follow-up once the converter files are gone. - Found but not removed here: the windows shim's `ReadWithoutLaunch` mode (~110 lines, overlaps #36200), `node_quic_binding.rs` constants JS never destructures (~35 lines), the native `NodeJSFS.unwatchFile` / `FSWatcher.hasRef` / `QuicSession.silentClose` / `QuicEndpoint.ref` bindings JS never calls, never-constructed `bun_install::Error` variants, and ~160 lines of `$`-declarations in `src/js/builtins.d.ts` with no users. <!-- robobun:evidence:begin --> --- **[review]** gate passed · iteration 1 · 172 files touched <details><summary>fails on main (without fix)</summary> ```console ASAN without fix: 10 failed, 320 skipped $ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/mechgate.xml" test/bundler/transpiler/react-compiler-fixtures.test.ts test/internal/source-lints/dead-symbols-react-compiler-webcore-idl-misc.test.ts bun test v1.4.0 (8326d1b) test/internal/source-lints/dead-symbols-react-compiler-webcore-idl-misc.test.ts: 88 | // Arena box alias with zero uses (HirVec is the one HIR actually uses). 89 | ["src/react_compiler/hir/mod.rs", /\bHirBox\b/], 90 | // Type predicate whose only callers were in the removed _exp validation. 91 | ["src/react_compiler/hir/mod.rs", /\bis_use_state_type\b/], 92 | ]; 93 | expect(resurrected(checks)).toEqual([]); ^ error: expect(received).toEqual(expected) - [] + [ + "src/react_compiler/validation/validate_no_derived_computations_in_effects.rs: validate_no_derived_computations_in_effects_exp", + "src/react_compiler/hir/environment_config.rs: validate_no_derived_computations_in_effects_exp", + "src/react_compiler/program.rs: validate_no_derived_computations_in_effects_exp", + "src/react_compiler/program.rs: \bSymbolHo ... (truncated) release without fix: 10 failed, 1146 skipped bun test v1.4.0-canary.1 (21a4206) test/internal/source-lints/dead-symbols-react-compiler-webcore-idl-misc.test.ts: 88 | // Arena box alias with zero uses (HirVec is the one HIR actually uses). 89 | ["src/react_compiler/hir/mod.rs", /\bHirBox\b/], 90 | // Type predicate whose only callers were in the removed _exp validation. 91 | ["src/react_compiler/hir/mod.rs", /\bis_use_state_type\b/], 92 | ]; 93 | expect(resurrected(checks)).toEqual([]); ^ error: expect(received).toEqual(expected) - [] + [ + "src/react_compiler/validation/validate_no_derived_computations_in_effects.rs: validate_no_derived_computations_in_effects_exp", + "src/react_compiler/hir/environment_config.rs: validate_no_derived_computations_in_effects_exp", + "src/react_compiler/program.rs: validate_no_derived_computations_in_effects_exp", + "src/react_compiler/program.rs: \bSymbolHost\b", + "src/react_compiler/lib.rs: \bSymbolHost\b", + "src/react_compiler/hir/mod.rs: \bHirBox\b", + "src/react_compiler/hir/mod.rs: \bis_use_state_type\b", + ] - Expected - 1 + Received + 9 at <anonymous> (/workspace/bun/test/internal/source ... (truncated) ``` </details> <details><summary>passes on PR (with fix)</summary> ```console ASAN with fix: 320 skipped $ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/mechgate.xml" test/bundler/transpiler/react-compiler-fixtures.test.ts test/internal/source-lints/dead-symbols-react-compiler-webcore-idl-misc.test.ts bun test v1.4.0 (8326d1b) test/internal/source-lints/dead-symbols-react-compiler-webcore-idl-misc.test.ts: (pass) dead react_compiler symbols do not reappear [21.23ms] (pass) dead exe_format symbols do not reappear [2.63ms] (pass) dead bun_core / bun_alloc / bun_ast / bun_ptr items do not reappear [18.66ms] (pass) dead bun_css items do not reappear [21.91ms] (pass) dead bun_jsc items do not reappear [20.67ms] (pass) dead FFI-crate items do not reappear [30.81ms] (pass) dead Rust symbols (install, webcore, jsc, leaf crates) do not reappear [18.03ms] (pass) unused re-export names do not reappear [12.35ms] (pass) stale build-script entries do not reappear [6.80ms] (pass) dead Rust FFI wrappers and trait methods do not reappear [22.18ms] (pass) dead C++ binding helpers do not reappear [99.82ms] (pass) dead WebCore / IDL binding code does not reappear [114.33ms] (pass) dead code in install_jsc, install_t ... (truncated) release with fix: 1146 skipped $ bun scripts/build.ts --profile=release [configured] bun-profile → bun (stripped) in 641ms (unchanged) ninja: Entering directory `/workspace/bun/build/release' [1/130] install /workspace/bun/src/node-fallbacks bun install v1.4.0-canary.1 (21a4206) Checked 111 installs across 104 packages (no changes) [3.00ms] [2/130] gen node-fallbacks/react-refresh.js Bundled 1 module in 5ms react-refresh.js 4.81 KB (entry point) [3/130] gen JSBuffer.lut.h Generating /workspace/bun/build/release/codegen/JSBuffer.lut.h from /workspace/bun/src/jsc/bindings/JSBuffer.cpp [4/130] gen generated_host_exports.rs generated_host_exports.rs: 92 exports (host=3, lazy=10, generic=79, rust=0); 241 extern-C blocks audited [5/130] gen node-fallbacks/*.js [6/130] gen cpp.rs (cppbind) [7/130] gen JS modules (bundle-modules) Preprocess modules (7887ms) Bundle modules (42ms) Postprocesss modules (259ms) Bundle Functions (695ms) Generate Code (32ms) [8.94s] Bundled "src/js" for production 2630 kb 197 internal modules 13 native modules 91 internal functions across 17 files [7/129] cargo bun_bin → libbun_rust.a (--target x86_64-unknown-linux-gnu) nightly-2026-07-20-x86_64-unknow ... (truncated) ``` </details> <details><summary>diff hotspot</summary> ``` scripts/build/config.ts | 4 - scripts/build/depVersionsHeader.ts | 3 - scripts/build/source.ts | 2 +- scripts/glob-sources.ts | 1 - src/ast/symbol.rs | 8 - src/boringssl_sys/boringssl.rs | 8 - src/bun_alloc/lib.rs | 29 +- src/bun_core/bounded_array.rs | 8 - src/bun_core/fmt.rs | 17 - src/bun_core/string/immutable.rs | 7 - src/bun_core/string/mod.rs | 11 - src/bun_core/string/write.rs | 3 - src/bun_core/windows_sys.rs | 2 +- src/cares_sys/c_ares.rs | 6 - src/clap/error.rs | 9 - src/clap/lib.rs | 5 - src/codegen/generate-js2native.ts | 1 - src/codegen/generate-unified-source-bundles.rb | 392 ------- src/csrf/lib.rs | 3 - src/css/css_parser.rs | 19 +- src/css/generics.rs | 10 - src/css/lib.rs | 2 +- src/css/properties/custom.rs | 8 +- src/css/rules/mod.rs | 7 +- src/css/values/angle.rs | 4 - src/css/values/calc.rs | 132 +-- src/css/values/css_string.rs | 6 - src/css/values/time.rs | 7 - src/css_derive/lib.rs | 4 +- src/css_jsc/css_internals.rs | 11 - src/errno/darwin_errno.rs | 1 - src/errno/freebsd_errno.rs | 1 - src/errno/linux_ ... (truncated) ``` </details> **gate history** · 4 passed · 1 rejected · iteration 1 <details><summary>evidence per changed file</summary> ``` file reads edits tests scripts/build/config.ts 0 0 0 scripts/build/depVersionsHeader.ts 0 0 0 scripts/build/source.ts 0 0 0 scripts/glob-sources.ts 0 0 0 src/ast/symbol.rs 0 0 0 src/boringssl_sys/boringssl.rs 0 0 0 src/bun_alloc/lib.rs 0 0 0 src/bun_core/bounded_array.rs 0 0 0 src/bun_core/fmt.rs 0 0 0 src/bun_core/string/immutable.rs 0 0 0 src/bun_core/string/mod.rs 0 0 0 src/bun_core/string/write.rs 0 0 0 src/bun_core/windows_sys.rs 0 0 0 src/cares_sys/c_ares.rs 0 0 0 src/clap/error.rs 0 0 0 src/clap/lib.rs 0 0 0 (+ 156 more files) ``` </details> <!-- robobun:evidence:end --> --------- Co-authored-by: Jarred Sumner <jarred@jarredsumner.com>
|
Closing as part of a cleanup of stale pull requests. This PR has had no new commits since 2026-07-28, it conflicts with main, and its last CI run failed. This is not a judgment on the fix itself. If the problem still reproduces on a current build, reopen this PR after a rebase or open a new one against main. |
2 participants
What
On Windows,
node_modules/.bincurrently holds two files per bin:<name>.exe(the embeddedbun_shim_implstub) and<name>.bunx(the per-bin metadata: relative target path + parsed shebang). This PR moves the metadata into a:bunxNTFS alternate data stream on the exe itself, so.binholds one file per bin and the metadata can't orphan or desync from its stub.The two-file layout remains the fallback when the stream can't be created (exFAT/FAT32 and some network shares reject named streams); readers probe the stream first and fall back to the sidecar.
Changes
Write (
bin.rscreate_windows_shim): write the exe first, then try<name>.exe:bunx(FILE_OVERWRITE_IF, so reinstalls truncate the stream without touching other streams). On success, delete any pre-existing<name>.bunxsidecar so upgrading from the old layout leaves nothing stale. On any stream-open error, write<name>.bunxinstead.Read, standalone shim (
bun_shim_impl.rs): copy the full image path (including.exe) intobuf1, append:bunx,NtCreateFile. On failure, overwriteexewithbunxin place and retry with the sidecar length. Theread_ptrwalk that derivesnode_modules/is driven byimage_path_b_lenalone and is unchanged by either suffix.Read,
bun.exefast path (run_command.rsBunXFastPath::try_launch): callers still build<...>.bunxinDIRECT_LAUNCH_BUFFER;try_launchrewrites the tail to.exe:bunxfor the probe, falls back to.bunx, then restores.bunxbefore handingbase_pathto the launcher (so the launcher's buffer math is unchanged).Unlink (
bin.rsunlink_bin_or_shim): no change. It already deletes both<name>.exeand<name>.bunx; deleting the exe removes its streams, and deleting the sidecar covers old-layout cleanup.Tests
toBeValidBin/toHaveBinsnow accept either layout. New Windows tests in the existingwindows bin linking shim should worksuite cover:.bincontains only.exeentries after install; every exe has a non-empty:bunxstream.bunxsidecar when no stream is presentbun install --forceover a project with stale.bunxsidecars removes themAll 47 tests in that suite pass on a Windows x64 debug build; the 3 new tests fail against
main'ssrc/.no test proof · iteration 1 · Platform-specific test(s) that do not run on this machine. Deferring to CI, which covers all platforms: test/cli/install/bun-install-lifecycle-scripts.test.ts test/cli/install/bun-install-registry.test.ts test/cli/install/bun-publish.test.ts test/cli/install/bun-run.test.ts test/cli/install/bunx.test.ts test/cli/install/isolated-install.test.ts test/regression/issue/13316.test.ts