fix(resync): re-diff the whole watch set when registration changes - #383
Merged
schickling-assistant merged 2 commits intoAug 29, 2026
Merged
Conversation
`refresh_watches` treated a registration change as affecting only the directories it registered, and polled just those. That holds for inotify, which adds and removes descriptors on a shared fd and keeps its queue, but not for notify's macOS FSEvents backend: `watch_inner`/`unwatch_inner` stop the single shared stream, the runloop teardown calls `FSEventsPurgeEventsForDeviceUpToEventId`, and `run()` restarts at `kFSEventStreamEventIdSinceNow`. Registering one new directory therefore destroys mutations already queued for directories that never left the set. A reconcile pass that admits a newly-valid seat registers its directories, so a carrier written just before that pass could lose its only notification with nothing left to re-read it: `poll_unwatched` skips it (its parent is still watched) and it is not under a newly registered directory. The transition was then never observed at all, which is what `compile_invalid_seat_does_not_block_existing_live_resync_watch` times out on for aarch64-darwin (#368) while passing on every Linux run. Measured on macOS 26.5 with notify 8.2.0: a directory watched throughout delivers its mutation 20/20 times on its own, and 18/20 when an unrelated second directory is registered immediately after the write. The same probe loses nothing on Linux. Delivery wins the race on an idle machine within 1ms, which is why a loaded 3-core CI runner sees this and a fast Mac does not. A changed registration is the same loss a backend error means, so it takes the same containment: re-read every subscribed carrier through the ordinary classified path. Digest equality keeps it silent when nothing moved, so only the recovered mutation emits. `poll_registered` had no other caller and is gone. The regression test is platform-neutral. It records the live parents as covered without handing them to the backend — the exact state a purge leaves, a registration that will never report what already happened — so it fails on Linux too without this change. Refs #368 agent-identity: dev3.direct.claude.paqjmjfq agent-persona: generalist agent-supervisor: unavailable agent-tool: Claude Code agent-tool-version: 2.1.250 agent-runtime: Claude Code 2.1.250 tooling-profile: dotfiles@a1a5f89
The FSEvents purge is in the attempt. `watch_inner` calls `stop()` — which tears down the runloop and purges the device's pending events — before `append_path` runs, so a `watch` that fails because the directory went missing since `dir_identity` looked at it costs exactly as many queued events as one that succeeds. `unwatch_inner` already had this shape and was already counted. Keying the rescan on `watch(..).is_ok()` therefore left one carrier with a missing parent directory purging every other agent's queued mutations on every pass, with nothing left to re-read them: a standing version of the same defect rather than the transient one. Refs #368 agent-identity: dev3.direct.claude.paqjmjfq agent-persona: generalist agent-supervisor: unavailable agent-tool: Claude Code agent-tool-version: 2.1.250 agent-runtime: Claude Code 2.1.250 tooling-profile: dotfiles@a1a5f89
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
check-aarch64-darwinis red onmain, so every PR's darwin gate is red — including#375, which is otherwise review-clean and linux-green. One test is responsible:
run::tests::compile_invalid_seat_does_not_block_existing_live_resync_watchtimes outwaiting for a resync event. It fails on darwin only, intermittently, and independently of
the diff under test; it has never failed on linux.
The cause is a lost event, not a slow one.
notify's macOS FSEvents backend keeps one shared stream for every watched path.watch_innerandunwatch_innerboth callstop()before mutating the path list, andrun()recreates the stream atsince_when: kFSEventStreamEventIdSinceNow— set once atconstruction and never advanced. (In 8.2.0 the runloop teardown also calls
FSEventsPurgeEventsForDeviceUpToEventId, making the discard explicit.) So registeringone new directory destroys mutations already queued for directories that never left the
watch set. Linux inotify adds and removes descriptors on a shared fd and keeps its queue,
which is why this is darwin-only.
refresh_watchesassumed the inotify semantics: it returned only the directories it hadjust registered, and
finish_carrier_updatere-read only those. So when a reconcile passadmitted a newly-valid seat, a carrier written moments earlier lost its only notification
with nothing left to recover it —
poll_unwatchedskips it because its parent is stillwatched, and it is not under a newly registered directory.
Measured with a standalone
notifyprobe (watcha, write ina, register unrelatedb,wait 10s), 100 trials per column on macOS 26.5 arm64:
The same probe loses nothing on linux. Nothing in it ever touches
b— registeringbisthe entire cause.
Goal
Darwin green on
main, and a change to the watch registration set can no longer silentlylose a carrier mutation on any platform.
Preview of the effect
A carrier changes, and the next reconcile pass admits a newly-valid seat, which registers
that seat's directories:
Before, the queued notification is discarded by the stream restart and nothing re-reads the
carrier, so the agent is never told its resource changed. After, the changed registration
re-diffs every subscribed carrier, the digest difference is observed, and the transition
emits as it would have. A carrier that did not change stays silent, because equal digests
emit nothing.
Decisions
A timeout increase was rejected. The event is destroyed, not delayed — the probe waits
10s per trial and still misses it, and there is no later pass to re-read the carrier. No
deadline could recover it.
A
cfg(target_os = "macos")gate was rejected. It would leave the recovery pathunexercised by the linux job, where nearly all of this repo's test signal comes from — a
correctness path that never runs under test is the defect it is meant to prevent. The
unconditional version costs one digest pass over the watch set, and only when a registration
actually changed.
The containment reuses what the module already states for a backend error: "may mean
mutation events were dropped ... equal states remain silent." A changed registration is the
same loss, so it takes the same response —
rescan_all()instead ofpoll_registered(®istered).poll_pathsnever emits directly; it marks dirty andschedules the carrier's ordinary class deadline.
The second commit counts the registration attempt, not its success — it is not a
refactor.
watch_innercallsstop(), and therefore purges, beforeappend_pathcanreject a directory that went missing since
dir_identitylooked at it. Keying the rescan onwatch(..).is_ok()would leave one carrier with a missing parent directory purging everyother agent's queued mutations on every pass, with nothing left to re-read them — a
standing version of this defect rather than the transient one.
unwatch_innerhas the sameshape and was already counted.
Verification
The new test
resync::tests::registering_another_directory_rediffs_the_untouched_watch_setis platform-neutral and deterministic. It models the purge rather than racing it: the live
seat's parent directories are recorded as covered but never handed to the backend — exactly
the state a purge leaves, a registration that will never report what already happened.
Proof it can fail: with the behavioural half reverted and the test kept, it fails on
linux; with the change, it passes. It also asserts the joining seat stays silent, so the
rescan is not merely emitting for everything it re-reads.
nix build .#st2on aarch64-darwin — the exact darwin CI gate — from its build log:The CI gate's exact
cargoTestFlagson x86_64-linux(
--workspace --lib --bins --test discovery --test codex_hooks --test hooks --test run --test driver_expansion): green.CI on this PR: green on both jobs, and
check-aarch64-darwinre-run repeatedly against thesame head to distinguish "fixed" from "lucky", since the test passed intermittently before.
Attempts 1-3 green; 4 and 5 in progress. Prior distribution for contrast: red on
main,failing on #366, 2 of 2 on #375, 1 pass on #374.
Complexity
No new abstraction, no new dependency, no new module boundary.
refresh_watcheschangesreturn type from
Vec<PathBuf>tobool, andpoll_registeredis deleted — it had noother caller. Net simplification of the call graph.
Concerns
install_liveruns per newly-live seat, so a cold boot of N seats now performs N digestpasses over a growing carrier set. Bounded by the number of bindings, and the same budget the
spec already accepts for degraded (watcher-less) mode, but it is a real behaviour change and
worth a reviewer's eye.
The rescan is unconditional rather than darwin-only, so linux pays for a defect it does not
have. That is deliberate — see Decisions — but it is a cost.
Friction & bottlenecks
and after this change, including under artificial CPU load. Delivery wins the race within
about a millisecond when the machine is idle, so only a loaded, low-core runner sees it.
That is why the reproduction above is a
notifyprobe rather than the test itself, and whythe CI re-runs are the end-to-end evidence.
gh run rerun --job <id>fails with "job cannot be rerun" when given a job id from anearlier attempt; the id changes every attempt and must be re-read from
runs/<id>/attempts/<n>/jobs.Follow-ups
Refs #384— on macOS, FSEvents reports realpaths whileWorker::carriersis keyed by thepath as given, so resync silently observes nothing whenever a catalog ancestor is a
symlink. Separate defect, separate fix, deliberately not bundled here; CI does not hit it
because a nix build's temp directory has no symlink component.
notifybehaviour is unfixed upstream and now reported:[Bug]: fsevent: registering a watch drops events already queued for the paths already being watched notify-rs/notify#993, with a runnable minimal reproduction at
https://github.com/schickling-repros/2026-08-notify-fsevent-watch-drops-queued-events.
8.2.0 is the latest stable (9.0.0-rc.4 is a release candidate and still loses events at
the same rate), so there is nothing to bump to and this containment is load-bearing rather
than belt-and-braces. Upstream #988 reports the same code as an O(N²) performance problem
and #937 is an adjacent but different symptom; neither reports the event loss.
References
Closes #368Refs #384Posted on behalf of @schickling
agent_identitysessionagent_personaagent_supervisoragent_toolagent_tool_versionagent_runtimetooling_profile