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] 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; + }; } }