Summary
I found a debounce bug in the app-server fs/watch notification path. The local debounce wrapper in app-server/src/fs_watch.rs stores the next deadline in next_allowance and initializes it only once. After the first event batch is emitted, later batches reuse that stale Instant, so subsequent filesystem changes can be emitted immediately instead of being debounced.
The upstream repository currently only allows collaborators to open pull requests, so I pushed a fix to my fork instead:
Impact
The first burst of file watcher events is coalesced correctly, but later bursts may bypass the intended debounce window. In the app-server path this affects fs/changed notifications, where the debounce interval is currently 200ms.
In practical terms, after the first notification burst, clients can receive noisier and less stable fs/changed notifications than intended.
Fix in the fork
The forked branch:
- moves the debouncing wrapper into
codex-file-watcher as DebouncedWatchReceiver
- resets the debounce deadline for each event batch
- uses deterministic path ordering via
BTreeSet
- flushes pending paths if the watcher channel closes during a debounce window
- updates app-server
fs/watch to use the shared debounce receiver
Regression coverage
I added a unit test with a 50ms debounce interval that verifies the second event batch is still debounced:
- send
a
- wait 25ms
- send
b
- receive one batch
[a, b]
- send
c
- assert
recv() does not return within 25ms
- send
d
- receive one batch
[c, d]
Before the fix, step 6 can fail because the stale deadline is already in the past.
I also added coverage for flushing pending debounced paths when the sender closes.
Validation
Ran locally:
just fmt
PATH="$HOME/.cargo/bin:$PATH" just test -p codex-file-watcher
PATH="$HOME/.cargo/bin:$PATH" just test -p codex-app-server fs_watch
Results:
codex-file-watcher: 21 tests passedcodex-app-server fs_watch: 8 tests passed, 777 skipped- the
bench-smoke step run by just test also passed
One unrelated existing warning appeared from codex-exec-server/src/client.rs about an unused variable.
Summary
I found a debounce bug in the app-server
fs/watchnotification path. The local debounce wrapper inapp-server/src/fs_watch.rsstores the next deadline innext_allowanceand initializes it only once. After the first event batch is emitted, later batches reuse that staleInstant, so subsequent filesystem changes can be emitted immediately instead of being debounced.The upstream repository currently only allows collaborators to open pull requests, so I pushed a fix to my fork instead:
Impact
The first burst of file watcher events is coalesced correctly, but later bursts may bypass the intended debounce window. In the app-server path this affects
fs/changednotifications, where the debounce interval is currently 200ms.In practical terms, after the first notification burst, clients can receive noisier and less stable
fs/changednotifications than intended.Fix in the fork
The forked branch:
codex-file-watcherasDebouncedWatchReceiverBTreeSetfs/watchto use the shared debounce receiverRegression coverage
I added a unit test with a 50ms debounce interval that verifies the second event batch is still debounced:
ab[a, b]crecv()does not return within 25msd[c, d]Before the fix, step 6 can fail because the stale deadline is already in the past.
I also added coverage for flushing pending debounced paths when the sender closes.
Validation
Ran locally:
Results:
codex-file-watcher: 21 tests passedcodex-app-server fs_watch: 8 tests passed, 777 skippedbench-smokestep run byjust testalso passedOne unrelated existing warning appeared from
codex-exec-server/src/client.rsabout an unused variable.