Fix inotify for directory watches by evanrittenhouse · Pull Request #25236 · oven-sh/bun

evanrittenhouse · 2025-11-29T23:35:25Z

What does this PR do?

There are two core changes to how we use inotify itself:

Register IN_MODIFY interest when watching a directory. This gives access to change events on files inside the watched directory. Because we want access to these events, this also removes the blanket rename casting in the directory register path.
Treat the IN_CREATE bit as rename. This is to preserve existing behavior from when we casted all events in the directory registration path to rename.

This also changes the core logic for consecutive duplicate event detection. The previous directory watch implementation would emit events if current_event.type != last_event.type AND current_event.hash != last_event.hash. This was fine previously since we didn't register IN_MODIFY and thus would cast all events on a watched directory to a rename. Now that we register with IN_MODIFY, we have to pick up change events as well. If we emit a rename followed by a change for the same file, under the previous logic we'd miss the change since the hash is based on the file path.

Lastly, this properly updates the last event's hash. It seems that got lost in a refactor (see 006575a).

NOTE: this DOES NOT YET start picking up consecutive duplicate events

TODO:

Remove consecutive event deduplication
Run new file is not being watched in fs.watch #3657 repro script(s)
Run fs.watch stops emitting change events for a file after it is deleted and recreated (Linux: WSL2/Ubuntu) #24875 repro script

How did you verify your code works?

Wrote a new test

coderabbitai · 2025-12-01T16:34:02Z

Walkthrough

Updates file-watcher behavior: choose .rename vs .change by operation, add hash-based deduplication and duplicate-suppression timing, include IN.MODIFY in inotify directory masks and treat CREATE like rename for certain events; adds a macOS fs.watch test for create/remove cycles.

Changes

Cohort / File(s)	Summary
Path watcher event emission `src/bun.js/node/path_watcher.zig`	Replace fixed `.rename` with conditional `.rename` for delete/rename/move_to and `.change` otherwise; compute event hash and assign to `last_change_event.hash`; revise duplicate-suppression logic to compare time_diff and combined event_type/hash equality; update `last_change_event.time_stamp`/`event_type`/`hash` together and note that `emit()` filters duplicates before flushing.
inotify watcher handling `src/watcher/INotifyWatcher.zig`	Add `IN.MODIFY` to directory watch mask; treat `IN.CREATE` similarly to move/rename when mapping inotify events to rename semantics in `watchEventFromInotifyEvent`.
Tests — fs.watch (macOS) `test/js/node/watch/fs.watch.test.ts`	Add macOS-guarded test that watches a newly created directory, repeatedly writes and removes `watch.txt`, and asserts receipt of change and rename events; mirrors existing watcher lifecycle and cleanup.

Possibly related PRs

fix(watcher): handle vim atomic save race on macOS #23566 — Modifies watcher event interpretation and macOS rename/create handling; strongly related to rename vs change semantics and deduplication adjustments.
Add BUN_WATCHER_TRACE environment variable for debugging file watcher events #23533 — Changes inotify processing paths and masks; overlaps with IN_MODIFY/mapping changes in the inotify watcher.

Pre-merge checks

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title 'Fix inotify for directory watches' is clear, specific, and accurately summarizes the main change in the PR: fixing inotify behavior for directory watches.
Description check	✅ Passed	The pull request description covers both required sections (what it does and how it was verified) with detailed technical explanations.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

coderabbitai

Actionable comments posted: 2

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Disabled knowledge base sources:

Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 9ca8de6 and 3a91a2a0c6fd1afb0069151a3bd58a1f1e5068f1.

📒 Files selected for processing (3)

src/bun.js/node/path_watcher.zig (3 hunks)
src/watcher/INotifyWatcher.zig (2 hunks)
test/js/node/watch/fs.watch.test.ts (1 hunks)

🧰 Additional context used

📓 Path-based instructions (9)

src/**/*.{cpp,zig}

📄 CodeRabbit inference engine (.cursor/rules/building-bun.mdc)

src/**/*.{cpp,zig}: Use bun bd or bun run build:debug to build debug versions for C++ and Zig source files; creates debug build at ./build/debug/bun-debug
Run tests using bun bd test <test-file> with the debug build; never use bun test directly as it will not include your changes
Execute files using bun bd <file> <...args>; never use bun <file> directly as it will not include your changes
Enable debug logs for specific scopes using BUN_DEBUG_$(SCOPE)=1 environment variable
Code generation happens automatically as part of the build process; no manual code generation commands are required

Files:

src/bun.js/node/path_watcher.zig
src/watcher/INotifyWatcher.zig

src/**/*.zig

📄 CodeRabbit inference engine (.cursor/rules/building-bun.mdc)

Use bun.Output.scoped(.${SCOPE}, .hidden) for creating debug logs in Zig code

Implement core functionality in Zig, typically in its own directory in src/

src/**/*.zig: Zig source code should be organized in src/*.zig for core runtime, JavaScript bindings, and package manager
In Zig code, be careful with allocators and use defer for cleanup to manage memory properly
Use BUN_DEBUG_QUIET_LOGS=1 to disable debug logging, or BUN_DEBUG_<scopeName>=1 to enable specific Output.scoped calls in debug builds
Run bun run zig:check-all to compile the Zig code on all platforms when making platform-specific changes

src/**/*.zig: Private fields in Zig are fully supported using the # prefix: struct { #foo: u32 };
Use decl literals in Zig for declaration initialization: const decl: Decl = .{ .binding = 0, .value = 0 };
Prefer @import at the bottom of the file (auto formatter will move them automatically)

Files:

src/bun.js/node/path_watcher.zig
src/watcher/INotifyWatcher.zig

**/*.zig

📄 CodeRabbit inference engine (.cursor/rules/zig-javascriptcore-classes.mdc)

**/*.zig: Expose generated bindings in Zig structs using pub const js = JSC.Codegen.JS<ClassName> with trait conversion methods: toJS, fromJS, and fromJSDirect
Use consistent parameter name globalObject instead of ctx in Zig constructor and method implementations
Use bun.JSError!JSValue return type for Zig methods and constructors to enable proper error handling and exception propagation
Implement resource cleanup using deinit() method that releases resources, followed by finalize() called by the GC that invokes deinit() and frees the pointer
Use JSC.markBinding(@src()) in finalize methods for debugging purposes before calling deinit()
For methods returning cached properties in Zig, declare external C++ functions using extern fn and callconv(JSC.conv) calling convention
Implement getter functions with naming pattern get<PropertyName> in Zig that accept this and globalObject parameters and return JSC.JSValue
Access JavaScript CallFrame arguments using callFrame.argument(i), check argument count with callFrame.argumentCount(), and get this with callFrame.thisValue()
For reference-counted objects, use .deref() in finalize instead of destroy() to release references to other JS objects

Files:

src/bun.js/node/path_watcher.zig
src/watcher/INotifyWatcher.zig

src/**/*.{ts,zig,cpp}

📄 CodeRabbit inference engine (CLAUDE.md)

Always use absolute paths in file operations

Files:

src/bun.js/node/path_watcher.zig
src/watcher/INotifyWatcher.zig

src/**/*.{ts,zig}

📄 CodeRabbit inference engine (CLAUDE.md)

Avoid shell commands in code - don't use find or grep; use Bun's Glob and built-in tools instead

Files:

src/bun.js/node/path_watcher.zig
src/watcher/INotifyWatcher.zig

test/**/*.{js,ts,jsx,tsx}

📄 CodeRabbit inference engine (.cursor/rules/writing-tests.mdc)

test/**/*.{js,ts,jsx,tsx}: Write tests as JavaScript and TypeScript files using Jest-style APIs (test, describe, expect) and import from bun:test
Use test.each and data-driven tests to reduce boilerplate when testing multiple similar cases

Files:

test/js/node/watch/fs.watch.test.ts

test/js/node/**/*.test.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

For Node.js compatibility tests, use the test/js/node/ directory

Files:

test/js/node/watch/fs.watch.test.ts

**/*.test.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.test.{ts,tsx}: For single-file tests, prefer using -e flag over tempDir
For multi-file tests, prefer using tempDir from harness and Bun.spawn over other temporary directory creation methods
Always use port: 0 for network tests and do not hardcode ports or use custom random port number functions
Use normalizeBunSnapshot to normalize snapshot output of tests
Never write tests that check for no 'panic' or 'uncaught exception' or similar in the test output - that is NOT a valid test
Use tempDir from harness to create temporary directories, do not use tmpdirSync or fs.mkdtempSync
When spawning processes in tests, check stdout/stderr expectations BEFORE checking exit code to get more useful error messages on test failure
Do not write flaky tests - do not use setTimeout in tests, instead await the condition to be met as you are testing the CONDITION not the TIME PASSING
Verify your test fails with USE_SYSTEM_BUN=1 bun test <file> and passes with bun bd test <file> - your test is NOT VALID if it passes with USE_SYSTEM_BUN=1

Files:

test/js/node/watch/fs.watch.test.ts

test/**/*.test.{ts,js,jsx,tsx,mjs,cjs}

📄 CodeRabbit inference engine (test/CLAUDE.md)

test/**/*.test.{ts,js,jsx,tsx,mjs,cjs}: Use bun:test with files that end in *.test.{ts,js,jsx,tsx,mjs,cjs}
Do not write flaky tests. Never wait for time to pass in tests; always wait for the condition to be met instead of using an arbitrary amount of time
Never use hardcoded port numbers in tests. Always use port: 0 to get a random port
Prefer concurrent tests over sequential tests using test.concurrent or describe.concurrent when multiple tests spawn processes or write files, unless it's very difficult to make them concurrent
When spawning Bun processes in tests, use bunExe and bunEnv from harness to ensure the same build of Bun is used and debug logging is silenced
Use -e flag for single-file tests when spawning Bun processes
Use tempDir() from harness to create temporary directories with files for multi-file tests instead of creating files manually
Prefer async/await over callbacks in tests
When callbacks must be used and it's just a single callback, use Promise.withResolvers to create a promise that can be resolved or rejected from a callback
Do not set a timeout on tests. Bun already has timeouts
Use Buffer.alloc(count, fill).toString() instead of 'A'.repeat(count) to create repetitive strings in tests, as ''.repeat is very slow in debug JavaScriptCore builds
Use describe blocks for grouping related tests
Always use await using or using to ensure proper resource cleanup in tests for APIs like Bun.listen, Bun.connect, Bun.spawn, Bun.serve, etc
Always check exit codes and test error scenarios in error tests
Use describe.each() for parameterized tests
Use toMatchSnapshot() for snapshot testing
Use beforeAll(), afterEach(), beforeEach() for setup/teardown in tests
Track resources (servers, clients) in arrays for cleanup in afterEach()

Files:

test/js/node/watch/fs.watch.test.ts

🧠 Learnings (14)

📚 Learning: 2025-11-24T18:35:08.612Z

Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/dev-server-tests.mdc:0-0
Timestamp: 2025-11-24T18:35:08.612Z
Learning: Applies to test/bake/**/*.test.ts : Use `dev.write()`, `dev.patch()`, and `dev.delete()` to mutate the filesystem instead of `node:fs` APIs, as dev server functions are hooked to wait for hot-reload and notify clients

Applied to files:

test/js/node/watch/fs.watch.test.ts

📚 Learning: 2025-11-24T18:36:33.069Z

Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T18:36:33.069Z
Learning: Applies to test/js/node/**/*.test.{ts,tsx} : For Node.js compatibility tests, use the `test/js/node/` directory

Applied to files:

test/js/node/watch/fs.watch.test.ts

📚 Learning: 2025-11-24T18:37:30.259Z

Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:30.259Z
Learning: Applies to test/**/*.test.{ts,js,jsx,tsx,mjs,cjs} : Use `tempDir()` from harness to create temporary directories with files for multi-file tests instead of creating files manually

Applied to files:

test/js/node/watch/fs.watch.test.ts

📚 Learning: 2025-11-24T18:36:33.069Z

Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T18:36:33.069Z
Learning: Applies to **/*.test.{ts,tsx} : For single-file tests, prefer using `-e` flag over `tempDir`

Applied to files:

test/js/node/watch/fs.watch.test.ts

📚 Learning: 2025-11-24T18:36:33.069Z

Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T18:36:33.069Z
Learning: Applies to **/*.test.{ts,tsx} : For multi-file tests, prefer using `tempDir` from `harness` and `Bun.spawn` over other temporary directory creation methods

Applied to files:

test/js/node/watch/fs.watch.test.ts

📚 Learning: 2025-11-24T18:36:33.069Z

Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T18:36:33.069Z
Learning: Applies to test/napi/**/*.test.{ts,tsx} : For N-API compatibility tests, use the `test/napi/` directory

Applied to files:

test/js/node/watch/fs.watch.test.ts

📚 Learning: 2025-11-24T18:36:33.069Z

Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T18:36:33.069Z
Learning: Applies to test/v8/**/*.test.{ts,tsx} : For V8 C++ API compatibility tests, use the `test/v8/` directory

Applied to files:

test/js/node/watch/fs.watch.test.ts

📚 Learning: 2025-11-24T18:37:30.259Z

Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:30.259Z
Learning: Applies to test/**/*.test.{ts,js,jsx,tsx,mjs,cjs} : Do not write flaky tests. Never wait for time to pass in tests; always wait for the condition to be met instead of using an arbitrary amount of time

Applied to files:

test/js/node/watch/fs.watch.test.ts

📚 Learning: 2025-11-24T18:36:33.069Z

Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T18:36:33.069Z
Learning: Applies to **/*.test.{ts,tsx} : Use `tempDir` from `harness` to create temporary directories, do not use `tmpdirSync` or `fs.mkdtempSync`

Applied to files:

test/js/node/watch/fs.watch.test.ts

📚 Learning: 2025-11-24T18:36:33.069Z

Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T18:36:33.069Z
Learning: Applies to test/js/web/**/*.test.{ts,tsx} : For Web API tests, use the `test/js/web/` directory (fetch, WebSocket, streams, etc.)

Applied to files:

test/js/node/watch/fs.watch.test.ts

📚 Learning: 2025-11-24T18:36:33.069Z

Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T18:36:33.069Z
Learning: Applies to test/js/bun/**/*.test.{ts,tsx} : For Bun-specific API tests, use the `test/js/bun/` directory (for http, crypto, ffi, shell, etc.)

Applied to files:

test/js/node/watch/fs.watch.test.ts

📚 Learning: 2025-11-24T18:37:30.259Z

Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:30.259Z
Learning: Applies to test/**/*.test.{ts,js,jsx,tsx,mjs,cjs} : Always use `await using` or `using` to ensure proper resource cleanup in tests for APIs like Bun.listen, Bun.connect, Bun.spawn, Bun.serve, etc

Applied to files:

test/js/node/watch/fs.watch.test.ts

📚 Learning: 2025-11-24T18:35:08.612Z

Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/dev-server-tests.mdc:0-0
Timestamp: 2025-11-24T18:35:08.612Z
Learning: Applies to test/bake/dev/esm.test.ts : Organize ESM tests in esm.test.ts for tests about various ESM features in development mode

Applied to files:

test/js/node/watch/fs.watch.test.ts

📚 Learning: 2025-11-24T18:37:30.259Z

Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:30.259Z
Learning: Applies to test/**/*.test.{ts,js,jsx,tsx,mjs,cjs} : Use `-e` flag for single-file tests when spawning Bun processes

Applied to files:

test/js/node/watch/fs.watch.test.ts

🔇 Additional comments (4)

src/watcher/INotifyWatcher.zig (2)

80-80: LGTM: Adding IN.MODIFY enables change events for directory watches.

This addition allows the watcher to receive modification events for files inside a watched directory, which is the core fix for this PR.

364-364: LGTM: IN_CREATE treated as rename aligns with Node.js fs.watch semantics.

File/folder creation in a watched directory emitting rename matches Node.js behavior and preserves prior semantics as stated in the PR description.

src/bun.js/node/path_watcher.zig (2)

250-251: LGTM: Conditional event_type enables proper change event emission.

The logic correctly differentiates between structural changes (delete/rename/move_to → rename) and content changes (modifications → change), which is the key fix for directory watches.

278-279: LGTM: Comment clarifies the deduplication responsibility.

There are two core changes to how we use inotify itself: 1. Register IN_MODIFY interest when watching a directory. This gives access to `change` events on files inside the watched directory. Because we want access to these events, this also removes the blanket `rename` casting in the directory register path. 2. Treat the `IN_CREATE` bit as `rename`. This is to preserve existing behavior from when we casted all events in the directory registration path to `rename`. This also changes the core logic for consecutive duplicate event detection. The previous directory watch implementation would emit events if `NOT (current_event.type != last_event.type AND current_event.hash != last_event.hash)`. Concretely, if you emitted `rename` then `change` on the same path, the event check would pass, but the hash one would mean the expression returns `False`. That means that the `NOT` block would evaluate to `True`, and we'd miss the event. This logic was fine previously since we didn't register IN_MODIFY and thus would cast all events on a watched directory to a `rename`, so the event types would always miss the check. Now that we register with IN_MODIFY, we have to pick up `change` events as well. Lastly, this properly updates the last event's hash. It seems that got lost in a refactor (see 006575a).

coderabbitai

Actionable comments posted: 1

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Disabled knowledge base sources:

Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 3a91a2a0c6fd1afb0069151a3bd58a1f1e5068f1 and 31c4808.

📒 Files selected for processing (3)

src/bun.js/node/path_watcher.zig (3 hunks)
src/watcher/INotifyWatcher.zig (2 hunks)
test/js/node/watch/fs.watch.test.ts (1 hunks)

🧰 Additional context used

📓 Path-based instructions (6)

test/**/*.{js,ts,jsx,tsx}

📄 CodeRabbit inference engine (.cursor/rules/writing-tests.mdc)

test/**/*.{js,ts,jsx,tsx}: Write tests as JavaScript and TypeScript files using Jest-style APIs (test, describe, expect) and import from bun:test
Use test.each and data-driven tests to reduce boilerplate when testing multiple similar cases

Files:

test/js/node/watch/fs.watch.test.ts

test/**/*.test.{ts,js,jsx,tsx,mjs,cjs}

📄 CodeRabbit inference engine (test/CLAUDE.md)

test/**/*.test.{ts,js,jsx,tsx,mjs,cjs}: Use bun:test with files that end in *.test.{ts,js,jsx,tsx,mjs,cjs}
Do not write flaky tests. Never wait for time to pass in tests; always wait for the condition to be met instead of using an arbitrary amount of time
Never use hardcoded port numbers in tests. Always use port: 0 to get a random port
Prefer concurrent tests over sequential tests using test.concurrent or describe.concurrent when multiple tests spawn processes or write files, unless it's very difficult to make them concurrent
When spawning Bun processes in tests, use bunExe and bunEnv from harness to ensure the same build of Bun is used and debug logging is silenced
Use -e flag for single-file tests when spawning Bun processes
Use tempDir() from harness to create temporary directories with files for multi-file tests instead of creating files manually
Prefer async/await over callbacks in tests
When callbacks must be used and it's just a single callback, use Promise.withResolvers to create a promise that can be resolved or rejected from a callback
Do not set a timeout on tests. Bun already has timeouts
Use Buffer.alloc(count, fill).toString() instead of 'A'.repeat(count) to create repetitive strings in tests, as ''.repeat is very slow in debug JavaScriptCore builds
Use describe blocks for grouping related tests
Always use await using or using to ensure proper resource cleanup in tests for APIs like Bun.listen, Bun.connect, Bun.spawn, Bun.serve, etc
Always check exit codes and test error scenarios in error tests
Use describe.each() for parameterized tests
Use toMatchSnapshot() for snapshot testing
Use beforeAll(), afterEach(), beforeEach() for setup/teardown in tests
Track resources (servers, clients) in arrays for cleanup in afterEach()

Files:

test/js/node/watch/fs.watch.test.ts

test/**/*.test.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

test/**/*.test.{ts,tsx}: For single-file tests in Bun test suite, prefer using -e flag over tempDir
For multi-file tests in Bun test suite, prefer using tempDir and Bun.spawn
Always use port: 0 when spawning servers in tests - do not hardcode ports or use custom random port functions
Use normalizeBunSnapshot to normalize snapshot output in tests instead of manual output comparison
Never write tests that check for no 'panic', 'uncaught exception', or similar strings in test output - that is not a valid test
Use tempDir from harness to create temporary directories in tests - do not use tmpdirSync or fs.mkdtempSync
In tests, call expect(stdout).toBe(...) before expect(exitCode).toBe(0) when spawning processes for more useful error messages on failure
Do not write flaky tests - do not use setTimeout in tests; instead await the condition to be met since you're testing the CONDITION, not TIME PASSING
Verify your test fails with USE_SYSTEM_BUN=1 bun test <file> and passes with bun bd test <file> - tests are not valid if they pass with USE_SYSTEM_BUN=1
Avoid shell commands in tests - do not use find or grep; use Bun's Glob and built-in tools instead
Test files must end in .test.ts or .test.tsx and be created in the appropriate test folder structure

Files:

test/js/node/watch/fs.watch.test.ts

src/**/*.{cpp,zig}

📄 CodeRabbit inference engine (.cursor/rules/building-bun.mdc)

src/**/*.{cpp,zig}: Use bun bd or bun run build:debug to build debug versions for C++ and Zig source files; creates debug build at ./build/debug/bun-debug
Run tests using bun bd test <test-file> with the debug build; never use bun test directly as it will not include your changes
Execute files using bun bd <file> <...args>; never use bun <file> directly as it will not include your changes
Enable debug logs for specific scopes using BUN_DEBUG_$(SCOPE)=1 environment variable
Code generation happens automatically as part of the build process; no manual code generation commands are required

Files:

src/watcher/INotifyWatcher.zig
src/bun.js/node/path_watcher.zig

src/**/*.zig

📄 CodeRabbit inference engine (.cursor/rules/building-bun.mdc)

Use bun.Output.scoped(.${SCOPE}, .hidden) for creating debug logs in Zig code

Implement core functionality in Zig, typically in its own directory in src/

src/**/*.zig: Private fields in Zig are fully supported using the # prefix: struct { #foo: u32 };
Use decl literals in Zig for declaration initialization: const decl: Decl = .{ .binding = 0, .value = 0 };
Prefer @import at the bottom of the file (auto formatter will move them automatically)

Be careful with memory management in Zig code - use defer for cleanup with allocators

Files:

src/watcher/INotifyWatcher.zig
src/bun.js/node/path_watcher.zig

**/*.zig

📄 CodeRabbit inference engine (.cursor/rules/zig-javascriptcore-classes.mdc)

**/*.zig: Expose generated bindings in Zig structs using pub const js = JSC.Codegen.JS<ClassName> with trait conversion methods: toJS, fromJS, and fromJSDirect
Use consistent parameter name globalObject instead of ctx in Zig constructor and method implementations
Use bun.JSError!JSValue return type for Zig methods and constructors to enable proper error handling and exception propagation
Implement resource cleanup using deinit() method that releases resources, followed by finalize() called by the GC that invokes deinit() and frees the pointer
Use JSC.markBinding(@src()) in finalize methods for debugging purposes before calling deinit()
For methods returning cached properties in Zig, declare external C++ functions using extern fn and callconv(JSC.conv) calling convention
Implement getter functions with naming pattern get<PropertyName> in Zig that accept this and globalObject parameters and return JSC.JSValue
Access JavaScript CallFrame arguments using callFrame.argument(i), check argument count with callFrame.argumentCount(), and get this with callFrame.thisValue()
For reference-counted objects, use .deref() in finalize instead of destroy() to release references to other JS objects

Files:

src/watcher/INotifyWatcher.zig
src/bun.js/node/path_watcher.zig

🧠 Learnings (17)

📚 Learning: 2025-11-24T18:35:08.612Z

Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/dev-server-tests.mdc:0-0
Timestamp: 2025-11-24T18:35:08.612Z
Learning: Applies to test/bake/**/*.test.ts : Use `dev.write()`, `dev.patch()`, and `dev.delete()` to mutate the filesystem instead of `node:fs` APIs, as dev server functions are hooked to wait for hot-reload and notify clients

Applied to files:

test/js/node/watch/fs.watch.test.ts

📚 Learning: 2025-11-24T18:37:30.259Z

Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:30.259Z
Learning: Applies to test/**/*.test.{ts,js,jsx,tsx,mjs,cjs} : Use `tempDir()` from harness to create temporary directories with files for multi-file tests instead of creating files manually

Applied to files:

test/js/node/watch/fs.watch.test.ts

📚 Learning: 2025-12-02T05:59:51.485Z

Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-02T05:59:51.485Z
Learning: Applies to test/**/*.test.{ts,tsx} : Use `tempDir` from `harness` to create temporary directories in tests - do not use `tmpdirSync` or `fs.mkdtempSync`

Applied to files:

test/js/node/watch/fs.watch.test.ts

📚 Learning: 2025-12-02T05:59:51.485Z

Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-02T05:59:51.485Z
Learning: Applies to test/**/*.test.{ts,tsx} : For single-file tests in Bun test suite, prefer using `-e` flag over `tempDir`

Applied to files:

test/js/node/watch/fs.watch.test.ts

📚 Learning: 2025-12-02T05:59:51.485Z

Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-02T05:59:51.485Z
Learning: Applies to test/**/*.test.{ts,tsx} : Avoid shell commands in tests - do not use `find` or `grep`; use Bun's Glob and built-in tools instead

Applied to files:

test/js/node/watch/fs.watch.test.ts

📚 Learning: 2025-11-24T18:37:30.259Z

Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:30.259Z
Learning: Applies to test/**/*.test.{ts,js,jsx,tsx,mjs,cjs} : Do not write flaky tests. Never wait for time to pass in tests; always wait for the condition to be met instead of using an arbitrary amount of time

Applied to files:

test/js/node/watch/fs.watch.test.ts

📚 Learning: 2025-12-02T05:59:51.485Z

Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-02T05:59:51.485Z
Learning: Applies to test/**/*.test.{ts,tsx} : For multi-file tests in Bun test suite, prefer using `tempDir` and `Bun.spawn`

Applied to files:

test/js/node/watch/fs.watch.test.ts

📚 Learning: 2025-12-02T05:59:51.485Z

Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-02T05:59:51.485Z
Learning: Applies to test/**/*.test.{ts,tsx} : Do not write flaky tests - do not use `setTimeout` in tests; instead `await` the condition to be met since you're testing the CONDITION, not TIME PASSING

Applied to files:

test/js/node/watch/fs.watch.test.ts

📚 Learning: 2025-11-07T17:28:51.204Z

Learnt from: markovejnovic
Repo: oven-sh/bun PR: 24423
File: test/js/node/path/browserify.test.js:472-475
Timestamp: 2025-11-07T17:28:51.204Z
Learning: In test files under test/js/node/path/, markovejnovic prefers functional programming patterns (e.g., reduce, map, filter chains) over imperative loops for clarity and readability.

Applied to files:

test/js/node/watch/fs.watch.test.ts

📚 Learning: 2025-11-24T18:37:30.259Z

Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:30.259Z
Learning: Applies to test/regression/issue/**/*.test.ts : Regression tests for specific issues go in `/test/regression/issue/${issueNumber}.test.ts`. Do not put tests without issue numbers in the regression directory

Applied to files:

test/js/node/watch/fs.watch.test.ts

📚 Learning: 2025-11-24T18:37:30.259Z

Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:30.259Z
Learning: Applies to test/**/*.test.{ts,js,jsx,tsx,mjs,cjs} : Use `-e` flag for single-file tests when spawning Bun processes

Applied to files:

test/js/node/watch/fs.watch.test.ts

📚 Learning: 2025-11-24T18:37:30.259Z

Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:30.259Z
Learning: Applies to test/**/*.test.{ts,js,jsx,tsx,mjs,cjs} : Always use `await using` or `using` to ensure proper resource cleanup in tests for APIs like Bun.listen, Bun.connect, Bun.spawn, Bun.serve, etc

Applied to files:

test/js/node/watch/fs.watch.test.ts

📚 Learning: 2025-11-24T18:35:08.612Z

Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/dev-server-tests.mdc:0-0
Timestamp: 2025-11-24T18:35:08.612Z
Learning: Applies to test/bake/dev/esm.test.ts : Organize ESM tests in esm.test.ts for tests about various ESM features in development mode

Applied to files:

test/js/node/watch/fs.watch.test.ts

📚 Learning: 2025-11-24T18:37:30.259Z

Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:30.259Z
Learning: Applies to test/**/*.test.{ts,js,jsx,tsx,mjs,cjs} : Always check exit codes and test error scenarios in error tests

Applied to files:

test/js/node/watch/fs.watch.test.ts

📚 Learning: 2025-09-02T19:17:26.376Z

Learnt from: taylordotfish
Repo: oven-sh/bun PR: 0
File: :0-0
Timestamp: 2025-09-02T19:17:26.376Z
Learning: In Bun's Zig codebase, when handling error unions where the same cleanup operation (like `rawFree`) needs to be performed regardless of success or failure, prefer using boolean folding with `else |err| switch (err)` over duplicating the cleanup call in multiple switch branches. This approach avoids code duplication while maintaining compile-time error checking.

Applied to files:

src/bun.js/node/path_watcher.zig

📚 Learning: 2025-09-30T22:53:19.887Z

Learnt from: pfgithub
Repo: oven-sh/bun PR: 23117
File: src/bun.js/test/snapshot.zig:265-276
Timestamp: 2025-09-30T22:53:19.887Z
Learning: In Bun's snapshot testing (src/bun.js/test/snapshot.zig), multiple inline snapshots at the same line and column (same call position) must have identical values. However, multiple inline snapshots on the same line at different columns are allowed to have different values. The check is position-specific (line+col), not line-wide.

Applied to files:

src/bun.js/node/path_watcher.zig

📚 Learning: 2025-10-24T10:43:09.398Z

Learnt from: fmguerreiro
Repo: oven-sh/bun PR: 23774
File: src/install/PackageManager/updatePackageJSONAndInstall.zig:548-548
Timestamp: 2025-10-24T10:43:09.398Z
Learning: In Bun's Zig codebase, the `as(usize, intCast(...))` cast pattern triggers a Zig compiler bug that causes compilation to hang indefinitely when used in complex control flow contexts (loops + short-circuit operators + optional unwrapping). Avoid this pattern and use simpler alternatives like just `intCast(...)` if type casting is necessary.

Applied to files:

src/bun.js/node/path_watcher.zig

🔇 Additional comments (6)

src/watcher/INotifyWatcher.zig (2)

80-80: LGTM! IN.MODIFY added to directory watch mask.

This change enables receiving modification events for files within watched directories, directly supporting the PR's goal of properly emitting change events for directory contents.

364-364: LGTM! IN.CREATE now signals rename.

This change treats creation events as rename signals, preserving prior behavior as documented in the PR objectives. This works in concert with the updated event_type logic in path_watcher.zig to correctly differentiate rename from change events.

src/bun.js/node/path_watcher.zig (4)

184-184: LGTM! Conditional event_type determination for file events.

The logic correctly maps delete/rename/move_to operations to .rename events and other operations (like write) to .change events. This refinement enables more accurate event categorization for file watches.

250-251: LGTM! Directory subpath events now differentiated by operation type.

This is the core change replacing the previous blanket .rename classification for directory events. The updated logic and comment clearly explain that rename/delete/move_to operations signal .rename, while other operations (like modify) signal .change. This enables proper change event emission for files within watched directories.

278-278: Helpful clarification comment.

This comment explains the deduplication behavior, making the code's intent clearer.

844-854: Duplicate detection logic correctly updated; hash assignment restored.

The updated condition now compares event_type alongside hash, which correctly allows sequences like rename followed by change for the same file path to pass through (per PR objectives). Line 854 restores the hash assignment that was lost in commit 006575a.

The logic is functionally correct: skips events if they're within 1ms of the previous event OR if they match both type and hash of the previous event.

Note: The past review suggestion about adding parentheses for operator precedence clarity (lines 844-846) remains applicable for readability, though the current logic is correct given Zig's precedence rules.

coderabbitai · 2025-12-03T16:28:10Z

+    const interval = repeat(() => {
+      const file = path.join(root, "watch.txt");
+      fs.writeFileSync(file, "hello");
+      fs.writeFileSync(file, "world");
+
+      fs.rmSync(file);
+    });


🧹 Nitpick | 🔵 Trivial

Test logic relies on interval looping rather than explicit file operations.

The test expects at least 2 change events but writes to the file twice per loop iteration (lines 244-245). According to the comment on line 219-220, consecutive changes aren't picked up, so the test only passes because the interval loops multiple times. This makes the test's behavior less explicit and potentially confusing.

Consider restructuring to make the intended behavior clearer—for instance, explicitly control the number of create/modify/delete cycles rather than relying on the interval timing to loop enough times, or adjust the assertions to match what actually happens in a single iteration.

🤖 Prompt for AI Agents

In test/js/node/watch/fs.watch.test.ts around lines 242 to 248, the test currently writes to the same file twice inside a repeat loop and relies on the interval looping to produce >=2 events; make the behavior explicit by removing the implicit reliance on repeat timing and either (A) perform a fixed number of explicit create/modify/delete cycles (e.g., loop N times) with a short wait between each operation long enough for the watcher to observe events, or (B) keep a single iteration but introduce an awaited delay between the two writeFileSync calls so consecutive changes are not coalesced; update the assertions to match the new explicit cycle count (expect events per cycle) or to assert at least the number produced by a single controlled iteration.

evanrittenhouse force-pushed the evanrittenhouse/inotify-changes branch 3 times, most recently from 9a256e5 to 3a91a2a Compare December 1, 2025 05:23

evanrittenhouse marked this pull request as ready for review December 1, 2025 16:29

coderabbitai Bot reviewed Dec 1, 2025

View reviewed changes

Comment thread src/bun.js/node/path_watcher.zig

Comment thread test/js/node/watch/fs.watch.test.ts

evanrittenhouse force-pushed the evanrittenhouse/inotify-changes branch from 3a91a2a to 31c4808 Compare December 3, 2025 16:24

coderabbitai Bot reviewed Dec 3, 2025

View reviewed changes

[autofix.ci] apply automated fixes

ff3ae0d

evanrittenhouse marked this pull request as draft December 10, 2025 19:12

github-actions Bot mentioned this pull request Apr 29, 2026

fs.watch: decouple from bun.Watcher, own inotify/FSEvents/kqueue directly #29952

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix inotify for directory watches#25236

Fix inotify for directory watches#25236
evanrittenhouse wants to merge 2 commits into
oven-sh:mainfrom
evanrittenhouse:evanrittenhouse/inotify-changes

evanrittenhouse commented Nov 29, 2025 •

edited

Loading

Uh oh!

coderabbitai Bot commented Dec 1, 2025 •

edited

Loading

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Uh oh!

coderabbitai Bot Dec 3, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

evanrittenhouse commented Nov 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What does this PR do?

How did you verify your code works?

Uh oh!

coderabbitai Bot commented Dec 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Possibly related PRs

Pre-merge checks

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

coderabbitai Bot Dec 3, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

evanrittenhouse commented Nov 29, 2025 •

edited

Loading

coderabbitai Bot commented Dec 1, 2025 •

edited

Loading