watcher(linux): drop inotify event on WatchItemIndex overflow instead of aborting

[robobun]

Two unguarded int casts in the file watcher turn what Zig handled tolerantly into a process abort from the FileWatcher thread (Cargo.toml sets panic = "abort", and try_from is a real runtime check not compiled out in release).

Site 1: kevent(2) returning -1 (macOS/FreeBSD)

src/watcher/KEventWatcher.rs:84-87: the coalesce guard is count < 64, so a kevent error return of -1 falls into usize::try_from(-1).expect("int cast") and aborts.

Already fixed in #31695 (uses the EINTR-retrying bun_sys::kevent wrapper so count is usize and real errors propagate as Err). Not touched here.

Site 2: watchlist position >= 65536 (Linux)

src/watcher/INotifyWatcher.rs:454:

Some(idx) => WatchItemIndex::try_from(idx).unwrap(),   // WatchItemIndex = u16

Nothing caps watchlist length (Watcher.rs:687/:783), so a project with >65535 watched paths can reach a position that doesn't fit in u16, and the .unwrap() aborts the process from the watcher thread. The Zig baseline (INotifyWatcher.zig:281) used @intCast, which is unchecked in Linux ReleaseFast builds, so the same input silently truncated and routed the event to the wrong entry. Wrong, but the process lived.

Fix: on overflow, scoped_log! and skip the event (events_processed += 1; continue;), matching the adjacent None arm for an unknown watch descriptor.

Verification

Source-level finding; reproducing requires >65535 inotify watches on a single module graph, which is impractical and flaky in CI (depends on fs.inotify.max_user_watches). cargo check -p bun_watcher and cargo clippy -p bun_watcher are clean, and test/cli/hot/watch-many-dirs.test.ts (the existing inotify stress test) still passes with the debug build.

[coderabbitai]

Warning

Review limit reached

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

Next review available in: 4 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: 2e53911e-fcd1-4b89-960e-88fc0371bdd4

📥 Commits

Reviewing files that changed from the base of the PR and between adbaf41 and e49c403.

📒 Files selected for processing (1)

• src/watcher/INotifyWatcher.rs

---

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

[robobun]

^{Updated 3:16 AM PT - Jul 16th, 2026}

❌ @robobun, your commit e49c403 has 3 failures in Build #73736 (All Failures):

• test/cli/run/require-cache.test.ts - code 1 on 🍎 26 aarch64
• test/cli/run/require-cache.test.ts - code 1 on 🍎 14 aarch64
• test/js/node/test/parallel/test-net-connect-memleak.js - code 1 on 🐧 3.23 x64
• test/js/node/test/parallel/test-net-connect-memleak.js - code 1 on 🐧 3.23 x64-baseline
• test/js/node/test/parallel/test-worker-message-port-transfer-terminate.js - SIGABRT on 🐧 13 x64-asan

---

🧪   To try this PR locally:

bunx bun-pr 34308

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

bun-34308 --bun

[robobun]

Why no regression test

The trigger condition is a watchlist position >= 65536, which requires bun --hot to import 65536+ modules. I probed both builds in this container (fs.inotify.max_user_watches = 1048576, so the kernel limit is not the blocker):

build	66000-module --hot initial load
release	~4.2 s
debug+ASAN (bun bd)	did not complete in 180 s

A fail-before/pass-after test has to run under bun bd, so the debug timing makes it infeasible there. A release-only test.skipIf(isDebug || isASAN) test would run in the CI release lanes but is skipped in exactly the environment the fail-before check runs in, so it cannot demonstrate the fix either.

The change itself is mechanical: .unwrap() on a u16::try_from(usize) becomes an Err arm that logs and skips (same control flow as the adjacent None arm). cargo check/clippy -p bun_watcher are clean and test/cli/hot/watch-many-dirs.test.ts still passes. Ready for a maintainer to merge on the strength of the source-level analysis.

[claude]

LGTM — the follow-up commit addresses the idx == u16::MAX sentinel collision I flagged, and the widened u32 dedup sentinel now matches WindowsWatcher.rs.

What was reviewed:

• Guard condition idx < WatchItemIndex::MAX as usize correctly excludes both ≥65536 and the NO_WATCH_ITEM (65535) reserved value; the as WatchItemIndex cast is bounded.
• last_event_id: u32 = u32::MAX with u32::from(u16) comparisons — lossless, no real index can collide, so the i=0 split_at_mut(0) path is unreachable.
• Drop path mirrors the adjacent None arm (events_processed += 1; continue), so batching bookkeeping stays consistent.
• No-test rationale (66k-module load infeasible under debug+ASAN) is documented in-thread; existing watch-many-dirs.test.ts still exercises the surrounding code.

Extended reasoning...

Overview

Two localized edits in src/watcher/INotifyWatcher.rs (Linux-only, cfg-gated): (1) replace WatchItemIndex::try_from(idx).unwrap() with a match guard that accepts idx < u16::MAX and otherwise logs via scoped_log! and skips the event, mirroring the adjacent unknown-watch-descriptor arm; (2) widen the dedup-loop sentinel last_event_id from WatchItemIndex (u16) to u32, matching the identical fix already present at WindowsWatcher.rs:515-517. Net diff is ~15 lines.

Security risks

None. This is defensive integer-bounds handling on an internal watchlist position derived from .iter().position(). No user-controlled input reaches a new sink; the change strictly narrows the set of inputs that reach downstream code and turns a process-abort into a logged event drop.

Level of scrutiny

Low-to-moderate. The trigger requires >65535 concurrently watched paths in a single --hot/--watch process — an extreme edge that the Zig baseline silently truncated and the Rust port turned into an abort. The change is purely defensive: for the <65535 common case, behavior is byte-identical (guard passes, sentinel comparison still works because u32::from(u16) is lossless). The only behavioral delta is at the boundary, where abort becomes drop-and-continue.

Other factors

• My earlier review on this PR flagged that the first revision still admitted idx == 65535, which collides with both NO_WATCH_ITEM and the old WatchItemIndex::MAX dedup sentinel (leading to head[0] on an empty slice at i=0). Commit e49c403 applied both suggested fixes: the guard now uses < (strict), and the sentinel is widened to u32::MAX with the same comment as the Windows sibling. I re-verified WatchItemIndex = u16 and NO_WATCH_ITEM = WatchItemIndex::MAX in Watcher.rs:48,51.
• The drop arm increments events_processed before continue, so the outer while events_processed < events.len() loop advances correctly and no event is double-processed or skipped.
• No automated regression test ships with this PR. CLAUDE.md normally requires one, but the author documented in-thread that reproducing needs 66k+ imported modules under bun bd, which did not complete in 180s under debug+ASAN. Given the fix is a mechanical bounds guard + sentinel widening that mirrors an already-landed sibling (WindowsWatcher.rs), and cargo check/clippy plus the existing inotify stress test pass, I think that rationale is acceptable for a maintainer to accept — but it's called out here so a human can override if they disagree.

[robobun]

CI on build 73736: the red lanes are unrelated to this diff. INotifyWatcher.rs is cfg(target_os = "linux")-gated and is not compiled into the darwin or windows binaries, so require-cache.test.ts (darwin aarch64, memory-leak threshold), test-net-connect-memleak.js (alpine), and test-worker-message-port-transfer-terminate.js (JSC exception-scope assert) cannot be caused here; all three are being tracked as main breaks. Everything else passed on retry. watch-many-dirs.test.ts flaked once on ubuntu-aarch64 with EISDIR/EBADF in the hot-reload file-read path, which this diff does not touch (the change is a no-op for watchlist positions below 65535, and that test uses ~260 entries).