Repository navigation
Conversation
…h/--hot On Docker bind mounts from a Windows/macOS host, WSL /mnt/... paths, and NFS/SMB shares, the native file-change APIs (inotify/kqueue/ ReadDirectoryChangesW) succeed but never deliver events, so --watch and --hot silently stop reloading. This adds an opt-in polling backend alongside the native one, selected at Watcher::init time via BUN_WATCHER_USE_POLLING=1 (interval configurable via BUN_WATCHER_POLL_INTERVAL, default 100ms). When enabled, the watcher thread periodically stat()s each file in the watchlist and emits a WRITE event when mtime/size/ino change, or DELETE when the file disappears. The baseline stat is captured at the moment add_file registers the path (same point where inotify would start listening), so a write that lands between registration and the first poll cycle is detected. The Watcher.platform field becomes a Backend enum with Native and Polling variants; the native watch_loop_cycle accesses the Platform via Backend::native()/native_mut(). No behaviour change when the env var is unset. Closes #5841
|
Warning Review limit reached
Next review available in: 8 minutes 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 (14)
Comment |
|
Updated 3:06 AM PT - Jul 29th, 2026
❌ @robobun, your commit 2d87a49 has 2 failures in
🧪 To try this PR locally: bunx bun-pr 36293That installs a local version of the PR into your bun-36293 --bun |
|
Found 2 issues this PR may fix:
🤖 Generated with Claude Code |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
When BUN_WATCHER_USE_POLLING is unset, statfs() the project root and switch to the polling backend when f_type indicates a filesystem that accepts inotify watches but never emits events for remote changes (9p/drvfs on WSL, NFS, SMB/CIFS, Parallels shared folders). Prints a one-line note so the user knows why. BUN_WATCHER_USE_POLLING=0 forces it off.
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
…OTDIR means gone; prune snapshots on eviction; inherit stderr in test
…o-detect
With { default: false }, get() returned Some(false) when unset (never None),
so the should_auto_poll arm in Watcher::init was unreachable.
Callers on macOS/FreeBSD were still opening per-file fds (and bailing on open failure) when polling is active, even though the polling backend stats by path and never uses the fd.
There was a problem hiding this comment.
No new issues found after the fixes in 17bea52…7ba9f93. This adds a new runtime-selected watcher backend (~250 lines of threaded polling logic, statfs auto-detection, changes to all three native platform watchers, and new user-facing env vars / stderr note) — substantial enough that a human should sign off on the API surface and the locking in polling::watch_loop_cycle.
What was reviewed:
Backend::native_mut()is only reachable via theBackend::Native(_)arm ofwatch_loop, so theunreachable!()is sound.PollingWatcher::register/unregisterand the diff loop all run underWatcher.mutex; the unlockedstat()phase only touches thread-local scratch.BUN_WATCHER_USE_POLLINGnow declared without a default, so unset →Noneandshould_auto_pollis reachable.requires_file_descriptors()wired through all four callers (jsc_hooks, bundle_v2, dev_server, add_file_by_path_slow); no remaining bareREQUIRES_FILE_DESCRIPTORSreads at call sites.
Extended reasoning...
Overview
Adds a stat-polling fallback backend for --watch/--hot/bun build --watch, selected at runtime via a new Backend enum on Watcher.platform. Selection is: explicit BUN_WATCHER_USE_POLLING={0,1} > Linux statfs(root).f_type against a hard-coded list of network-fs magics > native. New file PollingWatcher.rs (~250 lines) implements the poll loop; Watcher.rs gains the enum, backend dispatch in watch_loop, and gating of every kqueue/inotify registration on Backend::Native. All three platform watchers switch this.platform.<field> → this.platform.native_mut().<field>. A new requires_file_descriptors() runtime accessor replaces the compile-time constant at the four external call sites so polling on macOS doesn't open per-file O_EVTONLY fds. Two env vars, two doc pages, and a Linux-only LD_PRELOAD test are added.
Security risks
None identified. The new inputs are two process env vars read at watcher init; BUN_WATCHER_POLL_INTERVAL is clamped to .max(1) and parsed via the existing unsigned env-var kind (rejects overflow/non-numeric). should_auto_poll calls statfs on the project root only, and the magic-constant match is a closed allowlist. No user-controlled paths reach a syscall that wasn't already reachable via the native backend.
Level of scrutiny
High. This is core runtime infrastructure that runs on its own thread and interacts with the JS thread via Watcher.mutex. The change introduces runtime dispatch where there was previously a single compile-time backend, adds a stateful HashMap guarded by an existing mutex, and modifies cfg-gated code across three platforms. It also adds user-facing product behavior (auto-enabling polling with a stderr note on WSL/NFS) that is a UX/API decision. Four prior review rounds found and fixed one 🔴 (dropped events past MAX_COUNT), one 🔴 (dead auto-detect due to env-var default), and several 🟡s — all addressed — which itself indicates the change is non-trivial.
Other factors
All prior inline findings are resolved and verified in the current diff: the MAX_COUNT overflow now breaks before advancing the baseline; stat_path returns Option<Snapshot> and only ENOENT/ENOTDIR map to gone; flush_evictions calls p.unregister(hash); the env var has no default; requires_file_descriptors() is a runtime method threaded through all callers; the test uses stderr: "inherit"; and the docs note the newly-created-file limitation. The bug-hunting system found nothing new this run. Deferring rather than approving because the scope (new backend + cross-platform + threading + user-facing default-behavior change on Linux network mounts) exceeds what I'd auto-approve without a human weighing in on the API/UX choices.
There was a problem hiding this comment.
No further findings after the fixes in 17bea52 → 7ba9f93. This adds a new watcher backend with runtime selection, refactors Watcher.platform into a Backend enum across all three native impls, and introduces user-facing env vars plus a Linux statfs auto-detect heuristic — worth a human pass on the design and the cross-platform surface.
What was reviewed:
- Polling loop:
MAX_COUNToverflow now breaks before advancing baselines; onlyENOENT/ENOTDIRmap to deleted; snapshots pruned inflush_evictions. - Backend selection: dropping the env-var default makes the
should_auto_pollarm reachable;requires_file_descriptors()is now runtime-checked at all four call sites so polling on kqueue platforms skips the per-fileopen(). - Mutex discipline in
watch_loop_cycle: watchlist snapshot and diff both taken underWatcher.mutex, consistent withadd_file/flush_evictions; scratch buffers restored after dispatch. - Directory events are intentionally not emitted under polling — documented in watch-mode.mdx.
Extended reasoning...
Overview
Adds a stat-polling fallback backend for --watch/--hot/bun build --watch, selected at runtime via BUN_WATCHER_USE_POLLING or auto-detected on Linux via statfs().f_type against a list of filesystems (9p, NFS, SMB/CIFS, Parallels) where inotify accepts watches but never fires. Introduces a Backend { Native(Platform), Polling(PollingWatcher) } enum in place of the direct Platform field, threads native_mut() through the three platform watch_loop_cycle implementations, and adds a runtime requires_file_descriptors() accessor consumed by jsc_hooks.rs, bundle_v2.rs, dev_server/mod.rs, and add_file_by_path_slow. New file PollingWatcher.rs (~250 LoC) implements the sleep/snapshot/stat/diff cycle. Two new documented env vars, docs updates, and a Linux-only LD_PRELOAD test that stubs inotify_add_watch to simulate the WSL/bind-mount case.
Security risks
None identified. Inputs are the two env vars (boolean and unsigned, parsed through the existing env_var machinery with silent-fallback semantics; interval clamped to ≥1ms) and the project root path passed to statfs. No untrusted network/protocol data, no auth/crypto/permissions surface.
Level of scrutiny
High. This is core runtime infrastructure: the file watcher runs on its own thread and every --watch/--hot session depends on it. The change is not mechanical — it refactors a per-platform singleton into a runtime-selected enum, adds a new concurrent code path that shares Watcher.mutex with the native path, and introduces user-facing API surface (env-var names, default interval, the auto-detect fs-magic allowlist) that are design decisions a maintainer should sign off on. Four earlier review rounds each surfaced a real defect (baseline-advance-past-MAX_COUNT, dead auto-detect arm, unnecessary kqueue fds under polling, unpruned snapshot map), which is itself a signal that the change is non-trivial.
Other factors
- All seven prior findings were addressed and the threads resolved; the current diff reflects the fixes.
- The new test only covers Linux with
BUN_WATCHER_USE_POLLING=1explicitly set; the auto-detect path and the macOS/Windows polling code paths are compile-checked (cargo checkper the PR description) but not exercised in CI. - The polling backend deliberately does not emit
Kind::Directoryevents (documented limitation), soDirectoryWatchStore/bust_dir_cacherecovery is a no-op under polling — an accepted scope trade-off but one a maintainer should be aware of. native_mut()panics withunreachable!()if reached underBackend::Polling; all call sites are insideplatform::watch_loop_cyclewhichwatch_loop()only dispatches forBackend::Native, so this is sound today but is a coupling worth a human eye.
Given the scope, cross-platform reach, and new API surface, deferring rather than auto-approving.
|
CI on build 84799: all failures are unrelated to this diff (which only touches Build lanes (darwin-aarch64, x64-android): the Rust compile finished successfully ( Test lanes:
|
|
Closing as part of a cleanup of stale pull requests. This PR has had no new commits since 2026-07-29, it conflicts with main, and its last CI run failed. This is not a judgment on the fix itself. The linked issue (#5841) stays open. If the problem still reproduces on a current build, reopen this PR after a rebase or open a new one against main. |
|
@robobun let's get this one up again, in a new, fresh PR. |
|
Understood. I will rebase the change on the current main, run the tests again, and open a new PR. I will post the link here. |
|
The new PR is #42750. It is the same change, rebased on the current main (1400 commits later), with the tests run again on Linux and Windows. The PR body lists what changed since this one. |
3 participants
What
Adds a stat-polling backend for
--watch/--hot/bun build --watch, selected at runtime:/mnt/*, NFS, SMB/CIFS, Parallels shared folders), with a one-line note printed to stderr;BUN_WATCHER_USE_POLLING=1on any platform (interval viaBUN_WATCHER_POLL_INTERVAL, default 100 ms);BUN_WATCHER_USE_POLLING=0forces the native backend.Closes #5155. Closes #5841. Closes #14157.
Why
The native file-change APIs (inotify, kqueue,
ReadDirectoryChangesW) succeed but never deliver events on:/mnt/c/...paths ([WSL2] File changes made by Windows apps on Windows filesystem don't trigger notifications for Linux apps microsoft/WSL#4739)so
bun --watchsilently never reloads in those environments. Other tools expose the same escape hatch (nodemon -L, chokidarusePolling, tsxTSX_WATCH_USE_POLLING, webpackwatchOptions.poll); the auto-detection means WSL users get a working--watchwithout having to find the env var.How
Watcher.platformbecomes aBackendenum withNative(Platform)andPolling(PollingWatcher)variants, selected once inWatcher::init. The selection order isBUN_WATCHER_USE_POLLING(if set) >statfs(root).f_typeagainst a conservative allowlist (Linux only) > native. When polling:add_filecaptures a baselinestat()(mtime/size/ino) at registration timethis.mutex,stat()s each file, and emitsOp::WRITEwhen mtime/size/ino differ orOp::DELETEwhen the file is goneNo behaviour change when neither the env var nor the fs heuristic selects polling.
Verification
The test LD_PRELOADs a shim that makes
inotify_add_watchreturn a fake descriptor without registering anything, reproducing the WSL / bind-mount case where inotify reports success but never fires:Existing
watch.test.ts,hot.test.ts,watch-many-dirs.test.tsandwatcher-trace.test.tsstill pass;cargo check -p bun_watcherpasses on linux-x64, aarch64-apple-darwin, and x86_64-pc-windows-msvc.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/watch/watch.test.ts