From 7d7241170f52f478f7339a6ba682d270f6de7f51 Mon Sep 17 00:00:00 2001 From: CJ van den Berg Date: Mon, 30 Mar 2026 21:09:41 +0200 Subject: [PATCH 1/3] fix(fsevents): handle just the final state for coalesced events --- src/backend/FSEvents.zig | 29 ++++++++++++++++++++++------- src/nightwatch_test.zig | 13 +++++++++++-- 2 files changed, 33 insertions(+), 9 deletions(-) diff --git a/src/backend/FSEvents.zig b/src/backend/FSEvents.zig index d7316b6..deab2f3 100644 --- a/src/backend/FSEvents.zig +++ b/src/backend/FSEvents.zig @@ -229,21 +229,36 @@ fn callback( // an error back to the caller. Stopping the stream from inside the // callback would require a separate signal channel and is not worth // the complexity; the stream will keep delivering future events. - if (flags & kFSEventStreamEventFlagItemCreated != 0) { - ctx.handler.change(path, .created, ot) catch {}; - } + // FSEvents coalesces multiple operations into a single callback with + // multiple flags set. Processing all flags independently produces + // spurious duplicate events (e.g. ItemRemoved|ItemRenamed -> two + // deletes; ItemCreated|ItemRemoved -> spurious create before delete). + // + // Priority chain: Removed > Renamed > Modified > Created. + // Modified beats Created because FSEvents sets ItemCreated on writes to + // existing files when O_CREAT is used (e.g. `echo > file`), producing + // spurious create events. When both are set, the file already exists, + // so Modified is the accurate description. + // + // Exception: ItemRenamed|ItemModified on an existing path emits both + // created and modified, because a rename-in followed by a write can be + // coalesced by FSEvents into a single callback. if (flags & kFSEventStreamEventFlagItemRemoved != 0) { ctx.handler.change(path, .deleted, ot) catch {}; - } - if (flags & kFSEventStreamEventFlagItemRenamed != 0) { + } else if (flags & kFSEventStreamEventFlagItemRenamed != 0) { // FSEvents fires ItemRenamed for both sides of a rename unpaired. // Normalize to created/deleted based on whether the path still exists, // so move-in appears as created and move-out as deleted on all platforms. const exists = if (std.fs.accessAbsolute(path, .{})) |_| true else |_| false; ctx.handler.change(path, if (exists) .created else .deleted, ot) catch {}; - } - if (flags & kFSEventStreamEventFlagItemModified != 0) { + // If a write was coalesced with a move-in, also emit the modify. + if (exists and flags & kFSEventStreamEventFlagItemModified != 0) { + ctx.handler.change(path, .modified, ot) catch {}; + } + } else if (flags & kFSEventStreamEventFlagItemModified != 0) { ctx.handler.change(path, .modified, ot) catch {}; + } else if (flags & kFSEventStreamEventFlagItemCreated != 0) { + ctx.handler.change(path, .created, ot) catch {}; } } } diff --git a/src/nightwatch_test.zig b/src/nightwatch_test.zig index 6d4ed68..6ad7c11 100644 --- a/src/nightwatch_test.zig +++ b/src/nightwatch_test.zig @@ -234,6 +234,11 @@ fn testModifyFile(comptime Watcher: type, allocator: std.mem.Allocator) !void { var watcher = try Watcher.init(allocator, &th.handler); defer watcher.deinit(); try watcher.watch(tmp); + // Drain before writing: FSEvents may deliver a coalesced create+modify if the + // file was created just before the stream started. A drain here separates any + // stale creation event from the upcoming write, so the write arrives in its + // own callback with only ItemModified set. + try drainEvents(Watcher, &watcher); { const f = try std.fs.openFileAbsolute(file_path, .{ .mode = .write_only }); @@ -794,7 +799,9 @@ test "creating a file emits a 'created' event" { test "writing to a file emits a 'modified' event" { inline for (comptime std.enums.values(nw.Variant)) |variant| { - try testModifyFile(nw.Create(variant), std.testing.allocator); + testModifyFile(nw.Create(variant), std.testing.allocator) catch |e| { + if (e != error.SkipZigTest) return e; + }; } } @@ -860,7 +867,9 @@ test "rename: old-name event precedes new-name event" { test "rename-then-modify: rename event precedes the subsequent modify event" { inline for (comptime std.enums.values(nw.Variant)) |variant| { - try testRenameThenModify(nw.Create(variant), std.testing.allocator); + testRenameThenModify(nw.Create(variant), std.testing.allocator) catch |e| { + if (e != error.SkipZigTest) return e; + }; } } From 7171ad1905fc492a106617e2c917ce4e79fa9baa Mon Sep 17 00:00:00 2001 From: CJ van den Berg Date: Mon, 30 Mar 2026 21:46:08 +0200 Subject: [PATCH 2/3] fix(kqueue): fix handling of move-in subdirs --- src/backend/KQueue.zig | 10 ++++++++++ src/backend/KQueueDir.zig | 18 +++++++++++++++--- 2 files changed, 25 insertions(+), 3 deletions(-) diff --git a/src/backend/KQueue.zig b/src/backend/KQueue.zig index 542031c..8f3e717 100644 --- a/src/backend/KQueue.zig +++ b/src/backend/KQueue.zig @@ -171,6 +171,9 @@ fn thread_fn(self: *@This(), allocator: std.mem.Allocator) void { std.log.err("nightwatch: handler returned {s}, stopping watch thread", .{@errorName(e)}); return; }; + // Clean up snapshot so that a new dir at the same path is not + // skipped by scan_dir's snapshots.contains() check. + self.remove_watch(allocator, dir_path); } else if (ev.fflags & NOTE_RENAME != 0) { self.handler.change(dir_path, EventType.renamed, .dir) catch |e| { std.log.err("nightwatch: handler returned {s}, stopping watch thread", .{@errorName(e)}); @@ -285,6 +288,10 @@ fn scan_dir(self: *@This(), allocator: std.mem.Allocator, dir_path: []const u8) // a rename (old disappears, new appears) reports the source path before the dest. for (new_dirs.items) |full_path| { try self.handler.change(full_path, EventType.created, .dir); + // Start watching the moved-in dir so future changes inside it are detected + // and so its deletion fires NOTE_DELETE rather than being silently missed. + self.add_watch(allocator, full_path) catch |e| + std.log.err("nightwatch: add_watch on moved-in dir failed: {s}", .{@errorName(e)}); try self.emit_subtree_created(allocator, full_path); } for (to_delete.items) |name| { @@ -322,6 +329,9 @@ fn emit_subtree_created(self: *@This(), allocator: std.mem.Allocator, dir_path: if (ot == .file) { self.register_file_watch(allocator, full_path); } else { + // Watch nested subdirs so changes inside them are detected after move-in. + self.add_watch(allocator, full_path) catch |e| + std.log.err("nightwatch: add_watch on moved-in subdir failed: {s}", .{@errorName(e)}); try self.emit_subtree_created(allocator, full_path); } } diff --git a/src/backend/KQueueDir.zig b/src/backend/KQueueDir.zig index be082e7..ca95d63 100644 --- a/src/backend/KQueueDir.zig +++ b/src/backend/KQueueDir.zig @@ -152,6 +152,9 @@ fn thread_fn(self: *@This(), allocator: std.mem.Allocator) void { std.log.err("nightwatch: handler returned {s}, stopping watch thread", .{@errorName(e)}); return; }; + // Clean up snapshot so that a new dir at the same path is not + // skipped by scan_dir's snapshots.contains() check. + self.remove_watch(allocator, watch_path); } else if (ev.fflags & NOTE_RENAME != 0) { self.handler.change(watch_path, EventType.renamed, .dir) catch |e| { std.log.err("nightwatch: handler returned {s}, stopping watch thread", .{@errorName(e)}); @@ -279,7 +282,11 @@ fn scan_dir(self: *@This(), allocator: std.mem.Allocator, dir_path: []const u8) // Order: new dirs, deletions (source before dest for renames), creations, modifications. for (new_dirs.items) |full_path| { try self.handler.change(full_path, EventType.created, .dir); - try self.emit_subtree_created(full_path); + // Start watching the moved-in dir so future changes inside it are detected + // and so its deletion fires NOTE_DELETE rather than being silently missed. + self.add_watch(allocator, full_path) catch |e| + std.log.err("nightwatch: add_watch on moved-in dir failed: {s}", .{@errorName(e)}); + try self.emit_subtree_created(allocator, full_path); } for (to_delete.items) |name| { var path_buf: [std.fs.max_path_bytes]u8 = undefined; @@ -302,7 +309,7 @@ fn scan_dir(self: *@This(), allocator: std.mem.Allocator, dir_path: []const u8) // Walk dir_path recursively, emitting 'created' events for all contents. // Called after a new dir appears in scan_dir so callers see individual // 'created' events for the pre-existing tree of a moved-in directory. -fn emit_subtree_created(self: *@This(), dir_path: []const u8) error{HandlerFailed}!void { +fn emit_subtree_created(self: *@This(), allocator: std.mem.Allocator, dir_path: []const u8) !void { var dir = std.fs.openDirAbsolute(dir_path, .{ .iterate = true }) catch return; defer dir.close(); var iter = dir.iterate(); @@ -315,7 +322,12 @@ fn emit_subtree_created(self: *@This(), dir_path: []const u8) error{HandlerFaile var path_buf: [std.fs.max_path_bytes]u8 = undefined; const full_path = std.fmt.bufPrint(&path_buf, "{s}/{s}", .{ dir_path, entry.name }) catch continue; try self.handler.change(full_path, EventType.created, ot); - if (ot == .dir) try self.emit_subtree_created(full_path); + if (ot == .dir) { + // Watch nested subdirs so changes inside them are detected after move-in. + self.add_watch(allocator, full_path) catch |e| + std.log.err("nightwatch: add_watch on moved-in subdir failed: {s}", .{@errorName(e)}); + try self.emit_subtree_created(allocator, full_path); + } } } From b99d0e57a6147df82e741f751c36ce57b478761a Mon Sep 17 00:00:00 2001 From: CJ van den Berg Date: Mon, 30 Mar 2026 21:55:35 +0200 Subject: [PATCH 3/3] fix(kqueue): replace .renamed events with .deleted as they are now equivalent --- src/backend/KQueue.zig | 2 +- src/backend/KQueueDir.zig | 4 ++-- src/types.zig | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/backend/KQueue.zig b/src/backend/KQueue.zig index 8f3e717..7396bbf 100644 --- a/src/backend/KQueue.zig +++ b/src/backend/KQueue.zig @@ -175,7 +175,7 @@ fn thread_fn(self: *@This(), allocator: std.mem.Allocator) void { // skipped by scan_dir's snapshots.contains() check. self.remove_watch(allocator, dir_path); } else if (ev.fflags & NOTE_RENAME != 0) { - self.handler.change(dir_path, EventType.renamed, .dir) catch |e| { + self.handler.change(dir_path, EventType.deleted, .dir) catch |e| { std.log.err("nightwatch: handler returned {s}, stopping watch thread", .{@errorName(e)}); return; }; diff --git a/src/backend/KQueueDir.zig b/src/backend/KQueueDir.zig index ca95d63..4028ae2 100644 --- a/src/backend/KQueueDir.zig +++ b/src/backend/KQueueDir.zig @@ -136,7 +136,7 @@ fn thread_fn(self: *@This(), allocator: std.mem.Allocator) void { return; }; } else if (ev.fflags & NOTE_RENAME != 0) { - self.handler.change(watch_path, EventType.renamed, .file) catch |e| { + self.handler.change(watch_path, EventType.deleted, .file) catch |e| { std.log.err("nightwatch: handler returned {s}, stopping watch thread", .{@errorName(e)}); return; }; @@ -156,7 +156,7 @@ fn thread_fn(self: *@This(), allocator: std.mem.Allocator) void { // skipped by scan_dir's snapshots.contains() check. self.remove_watch(allocator, watch_path); } else if (ev.fflags & NOTE_RENAME != 0) { - self.handler.change(watch_path, EventType.renamed, .dir) catch |e| { + self.handler.change(watch_path, EventType.deleted, .dir) catch |e| { std.log.err("nightwatch: handler returned {s}, stopping watch thread", .{@errorName(e)}); return; }; diff --git a/src/types.zig b/src/types.zig index cf5695b..507211f 100644 --- a/src/types.zig +++ b/src/types.zig @@ -33,7 +33,7 @@ pub const EventType = enum { /// destination side. /// /// - **kqueue / kqueuedir**: when a watched *directory* is itself - /// renamed, a `renamed` change event is emitted for the old directory + /// renamed, a `deleted` change event is emitted for the old directory /// path (the new path is not known). Renames of *files inside* a /// watched directory are detected indirectly via directory-level /// `NOTE_WRITE` events and appear as `deleted` + `created`.