Skip to content

feat(filesystem): honor the non-recursive watch option - #17875

Open
EhabY wants to merge 2 commits into
eclipse-theia:masterfrom
EhabY:feat/non-recursive-file-watching
Open

feat(filesystem): honor the non-recursive watch option#17875
EhabY wants to merge 2 commits into
eclipse-theia:masterfrom
EhabY:feat/non-recursive-file-watching

Conversation

@EhabY

@EhabY EhabY commented Aug 2, 2026

Copy link
Copy Markdown
Contributor

What it does

Fixes #17700, supersedes the mitigation from #17633.

DiskFileSystemProvider.watch forwarded only { ignored }, so recursive was dropped and every request became a recursive @parcel/watcher subscription: a non-recursive watch rooted outside the workspace crawled every sibling subtree (#17632), and a watch on a single file crawled its parent.

recursive is honored now. Recursive requests still go to parcel; non-recursive ones go to a new NodeDirectoryWatcher that watches one directory level with fs.watch, and requests resolving to the same directory share one handle. MainFileSystemEventService no longer drops plugin watches rooted above a workspace root, which restores detection of a deleted workspace folder, the thing JDT-LS watches the parent for.

Stacked: based on #17943, refactored further by #17944. Bases stay master because the base branches live on a fork.

Decisions worth a look:

  • One watcher per directory, not per request. On macOS libuv rebuilds its single FSEventStream on every handle start or stop, dropping in-flight events of every other watcher in the process. Excludes are per request, so different excludes still share a handle.
  • Case-only renames go through an exact-case readdir, because stat accepts a differing case. Linux keeps the cheap stat. Mirrors VS Code's existsChildStrictCase.
  • Deletions are confirmed 100 ms late, so an atomic save is an update rather than a delete plus an add. Directory identity is dev + ino + birthtimeMs, because rm -rf dir && mkdir dir reuses the inode.
  • Events are processed even while no request is attached. Skipping, as the parcel watcher does, would leave the children snapshot and a pending deletion stale for a request arriving within the disposal grace period.
  • Failures are logged and retried, never sent to client.onError, which drops the URI and shows a one-shot "large workspace" toast. Parcel watcher failures surface as a one-shot, path-less "large workspace" warning and never retry #17922 tracks the parcel side.

Follows VS Code where settled: the 75 ms aggregation, the 100 ms deferred delete, NFC and case-insensitive matching, the /Volumes/ refusal (microsoft/vscode#106879). Diverges where VS Code drops events: a Windows ReadDirectoryChangesW overflow arrives as filename === null and is recovered by rescanning, and a lost directory is polled at 500 ms with its missed changes replayed.

Until #17943 merges this PR shows its commit too, so review just the feature commit: git diff HEAD~1...HEAD. Nothing moves, so no copy detection is needed. Where the +1574/−213 goes:

lines
nodejs-watcher/node-directory-watcher.ts +555 the event state machine, the only file needing close reading
tests +893 its spec (630), the routing/sharing spec (205), a shared helper (58)
parcel-watcher/parcel-filesystem-service.ts +122 routing by recursive, the renames with deprecated aliases
plugin-ext mitigation removal −174 main-file-system-event-service.ts and its spec (#17633)
plumbing and rename fallout ~40 the recursive flag in protocol and disk provider, DI module, imports, CHANGELOG

How to test

  1. Linux, workspace beside large siblings under a shared parent, with Red Hat Java (not LightWeight): inotify-consumers shows the siblings are no longer watched, and deleting the workspace folder is detected.
  2. Open several files in one folder; only one watch is created for it.
  3. npx lerna run test --scope @theia/filesystem runs 43 cases: 32 on the watcher, 11 on routing and sharing. A substituted watch handle drives what no host can reproduce, such as buffer overflow, inode reuse, decomposed names and refused handles. The with a real fs.watch handle block leaves platform truth to the CI matrix and is not platform-gated, so a regression on one host fails that host's job.

Follow-ups

#17944 puts both watcher kinds behind one interface and fixes defects found while auditing this code against VS Code; they are listed there.

Known limitations, none regressions: fs.watch is best-effort per Node's docs; a directory recreated reusing both inode and birth millisecond is not detected as replaced, the same gap as VS Code; macOS still rebuilds the FSEventStream on any watcher start or stop; Windows recovers additions and deletions after an overflow but not updates; events buffered at restart are dropped and the children diff recovers additions and deletions; excludes match case-sensitively against absolute paths, same as the parcel watcher, so it should change in both places or neither.

Breaking changes

  • This PR introduces breaking changes and requires careful review. If yes, the breaking changes section in the changelog has been updated.

[filesystem]: ParcelFileSystemWatcherService renamed to FileSystemWatcherServiceImpl and PacelWatcherHandle to WatcherHandle, both keeping deprecated aliases; the watchers map now holds a WatcherInstance of either kind, so overrides of createWatcher/getWatcherKey and readers of watchers must check the new signatures. [plugin-ext]: MainFileSystemEventService lost its workspaceService parameter and shouldSkipWatch method.

Attribution

Review checklist

Reminder for reviewers

@github-project-automation github-project-automation Bot moved this to Waiting on reviewers in PR Backlog Aug 2, 2026
@EhabY
EhabY force-pushed the feat/non-recursive-file-watching branch from 63af86d to 231cf47 Compare August 3, 2026 09:35
@EhabY
EhabY force-pushed the feat/non-recursive-file-watching branch from 231cf47 to 191d465 Compare August 16, 2026 13:10
@EhabY
EhabY force-pushed the feat/non-recursive-file-watching branch from 191d465 to 95fbdb7 Compare August 16, 2026 13:43
@EhabY EhabY changed the title feat(filesystem): support non-recursive file watching feat(filesystem): honor the non-recursive watch option Aug 16, 2026
@EhabY
EhabY force-pushed the feat/non-recursive-file-watching branch 14 times, most recently from d61246c to f329ff5 Compare August 17, 2026 10:26
@EhabY
EhabY force-pushed the feat/non-recursive-file-watching branch from f329ff5 to 4e37ef0 Compare August 23, 2026 14:27
On macOS, creating a child also modifies its parent, so FSEvents may or
may not additionally report the watched root. The exact-set assertion
therefore failed intermittently: on one commit, macos-15 node-24.x
passed while macos-15 node-22.x failed.

Assert that no unexpected path arrived instead, excluding the root. The
waitForChange calls above already establish that each expected URI
arrived.
@EhabY
EhabY force-pushed the feat/non-recursive-file-watching branch from 4e37ef0 to 1486eba Compare August 23, 2026 14:56
The backend dropped the recursive flag, so every watch became a recursive
parcel subscription: a non-recursive watch rooted outside the workspace
crawled every sibling subtree, and watching a single file crawled its parent
directory.

Non-recursive requests now go to a NodeDirectoryWatcher that watches one
directory level with fs.watch, and requests resolving to the same directory
share one handle. Sharing matters beyond saving descriptors: on macOS libuv
keeps one FSEventStream per event loop and rebuilds it whenever any handle
starts or stops, losing the events of every other watcher while it does.

Platform handling follows VS Code where it is settled: NFC normalization and
case-insensitive matching on macOS, an exact-case readdir so a rename that
only changes case is not reported as an update, the deferred delete that
survives atomic saves, and refusing to watch a macOS network share. It
diverges where VS Code drops events: a Windows ReadDirectoryChangesW overflow
arrives as a null file name and is recovered by rescanning the directory.

This removes the mitigation from eclipse-theia#17633, restoring detection of a workspace
folder deleted while a plugin watches its parent.

Closes eclipse-theia#17700
@EhabY
EhabY force-pushed the feat/non-recursive-file-watching branch from 1486eba to afb6b17 Compare August 23, 2026 15:36
@EhabY
EhabY marked this pull request as ready for review August 23, 2026 17:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Waiting on reviewers

Development

Successfully merging this pull request may close these issues.

Support first-class non-recursive file watching (à la VS Code)

1 participant