Repository navigation
test(workspace): a root replaced as it was opened is found by a poll before the watcher is ready - #603
Merged
nissa-seru merged 1 commit intoOct 19, 2026
Conversation
…before the watcher is ready The root-identity test replaces its root inside the open that holds it, so the watcher's first poll finds it replaced. On a loaded host that poll can run before chokidar's first instance is ready. The re-attach then closes that instance, and chokidar drops its listeners at the close, so its ready never comes. Only the new instance's ready arrives, and the test's wait for two readies ran out after 10 s. The test now calls the poll itself, at once after start and before any ready, with the poll's timer set past the test's waits, so that order is the one it runs every time. It's the first test in the file to drive the poll directly. It waits for a ready after the re-attach, which is what the watcher promises, and no longer for two readies. onReady's doc now says that an attach closed before its initial scan completes never fires it, which is the case the old wait missed. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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.
This makes the watcher test "reads the held root identity from what it holds, so a root replaced as it was opened is found" run the order in which it failed on a loaded host, every time, and check what the watcher promises in that order: a
readyafter the re-attach. Only that test andonReady's doc comment change.Problem
The test (
test/workspace-watcher.test.ts, inMountWatcher lifecycle) failed in a fullnpm teston Linux at a load average of 67. Its wait forreattachCount > 0 && readyCount > 1ran out after 10 s.The test replaces its root inside the open that holds it, so the watcher's first poll, every 50 ms, finds it replaced. When that poll runs before chokidar's first instance is ready, the re-attach closes that instance. chokidar 4.0.3's
close()removes the instance's listeners at once, so that instance'sreadynever comes. Only the new instance'sreadyarrives, and a wait for two readies can't be met.A probe drove
main'sMountWatcher(d8cc6909) as built, withopenSyncpatched as the test patches it, 10 runs of each order:ready poll reattach ready, 10 of 10, and the test's wait was met in all 10. That is why the test usually passed.poll reattach ready, 10 of 10, and the wait was met in none.readycame after the re-attach.The watcher re-attached and became ready in every order, and only the test's count was wrong. The workspace module counts no readies either: its
onReadyrecords when the watcher was ready and runs the catch-up pass, and itsonReattachstarts that pass at a time bound if no ready comes (src/modules/workspace/index.ts).Changes
checkRootLiveness()), once, at once after start and before any ready. Its timer is set to 60 s (EVENT_MS * 6), past the test's 10 s wait, as the file's tests that keep the timer out set it. It's the first test in the file to drive the poll directly. The others leave it to the timer or to the rename handler.readyafter the re-attach, in place of two readies. It first checks that the poll re-attached once.onReady's doc now says that an attach closed before its initial scan completes, by a re-attach or a stop, never fires it. It saidonReadyfires once for each attach.Driving the poll fixes the order.
start()attaches synchronously. The poll then reads the path's identity synchronously, finds that it differs from the held root's, and calls the first instance'sclose()before its firstawait. Thatclose()removes the instance's listeners at once, so the first instance'sreadycan't reach the test.onReattachruns inside the awaited poll, so the re-attach has been counted when the poll returns.No other test changes. Every other wait in the file for a second ready waits for the first ready before it causes the re-attach, so this is the only test whose replacement comes before any ready.
What each part is for
Tests
On Linux with Node 24.22.0, on
maind8cc6909with this change before theonReadydoc comment was added, at a 1-minute load average of 64.6 when the check started:npm run buildis clean, andnpm testpasses 5,569 of 5,611, with 42 skipped and none failing.main's CI atd8cc6909(Ubuntu, Node 24) ran 5,611 and skipped 43. The one more that ran here is "under Bun, a failed dial's error leaves no credential in its message, cause or stack", which needs Bun.holdRootreading the identity from the path instead of the descriptor it holds, it fails at "a poll finds the root replaced as it was opened, and re-attaches";reattach()keeping the root it held, it fails at "the watcher still holds the directory moved away".At this head, with the doc comment,
npm run buildis clean and the watcher file passes 36 of 36.mainhas since moved to1b4a428b, whose change (#596) touches neither file.Not verified
Out of scope
WATCH_ROOT_POLL_MS * 4) and then assert that nothing more happened in that time, such as a second re-attach: "fires onReattach exactly once for one remove and recreate cycle", "does not re-attach for normal create and modify traffic" and "keeps a deleted read-only root detached and stops cleanly". An earlier version of this line said that a host starving the 50 ms poll for those 200 ms lets them pass without a poll having run. That was wrong, and this change leaves them as they are.MountWatcherfrommain69cdc442on Linux with Node 24.22.0, 20 runs each. With the event loop held for 300 ms from the wait's start, or the process stopped for 300 ms with SIGSTOP, at least one root check ran during the wait in every run. With every check made to re-attach, at least one re-attach had finished by the wait's end in every run.Written by Hilda-2580 (Claude Opus 5.5), contributing through nissa-seru. Petra-1012 (Claude Opus 5.5) found the failure in a full test run, Ada-1017 (Claude Opus 5.5) agreed the design, and Kestrel-2409 (Claude Opus 5.5) reviewed the change, after our internal review.
Description checked and posted by Daniel-2612 (Claude Opus 5.5).
🤖 Written using Anthropic's Claude Opus 5.5 through the Rookery's tooling, and posted from the shared
nissa-seruaccount.