fix(workspace): the watcher holds its root on Linux, so an entry named like the root doesn't re-attach it

changelog.d/watcher-holds-root.breaking.md

@@ -0,0 +1,13 @@
+- **Operators:** on Linux, a workspace mount's watcher holds the mount's root
+  directory open while it watches it, so it can tell whether that directory
+  has been removed whatever has since been made at its path.
+  - **What stops working:** `umount` of the file system under a watched
+    root fails with "target is busy" while the host watches it. Stop the host
+    first, or use `umount -l`, which still detaches it.
+  - **What gets slower:** a root removed, or replaced by a rename over it, is
+    found at the watcher's next root poll (`watchRootPollMs`, 2 s by default)
+    rather than at once.
+  - **Unchanged:** where the watcher can't hold the root (macOS, Windows, or
+    on Linux a root it can't open as a directory), it holds nothing open, as
+    before. On WSL 2, Windows programs can still rename or remove a watched
+    folder on a Windows drive.

changelog.d/watcher-root-name-renames.fixed.md

@@ -0,0 +1,12 @@
+- On Linux, a workspace mount's watcher no longer re-attaches, rescanning
+  the whole mount and running its catch-up pass, when a file or directory
+  with the mount root's own name is made, saved over or removed anywhere in
+  the mount, as a program a build names after its directory is on every
+  build. libuv reports such a rename under the same name as the root's own
+  removal, and the watcher took every one for the root's. It now holds its
+  root open (see Breaking) and checks whether the root is still the directory
+  it holds. Where it can't hold the root, as on macOS and Windows or without
+  read permission, such a rename still re-attaches it, as before.
+- A workspace mount's watcher whose re-attach fails to close its old chokidar
+  instance now reports that and attaches again. It used to stop watching the
+  mount until the root's identity changed.

src/modules/workspace/watcher.ts

@@ -6,7 +6,7 @@
  */
 
 import { watch, type FSWatcher } from 'chokidar';
-import { mkdirSync, statSync } from 'node:fs';
+import { closeSync, constants, fstatSync, mkdirSync, openSync, statSync, type Stats } from 'node:fs';
 import { basename, resolve, sep } from 'node:path';
 import { passedOverAt } from './observe.js';
 import { type MountConfig } from './types.js';
@@ -43,6 +43,8 @@ interface RootIdentity {
   birthtimeMs: number;
 }
 
+const identityOf = (stat: Stats): RootIdentity => ({ dev: stat.dev, ino: stat.ino, birthtimeMs: stat.birthtimeMs });
+
 /**
  * Manages filesystem watching for a single mount.
  * Handles debouncing, ignore patterns, and write suppression.
@@ -58,13 +60,15 @@ export class MountWatcher {
   private suppressedPaths = new Set<string>();
   private suppressionTimers = new Map<string, ReturnType<typeof setTimeout>>();
   private rootIdentity: RootIdentity | null = null;
+  // The root as last attached, held open on Linux (see holdRoot).
+  private rootFd: number | null = null;
   private detached = false;
   private attaching = false;
   private stopped = true;
   private readonly debounceMs: number;
   private readonly config: MountConfig;
   /**
-   * The root as the watcher makes, watches and checks it, and as each
+   * The root as the watcher makes, holds, watches and checks it, and as each
    * event's path is read against: the configured path resolved (a link in it
    * kept), since chokidar hands back the paths it reports normalized, and
    * those of a root configured as `./m` matched none of them.
@@ -173,13 +177,19 @@ export class MountWatcher {
     this.watcher.on('error', (err) => {
       this.reportError(err);
     });
-    this.watcher.on('raw', (event: string, rawPath: string, _details: unknown) => {
-      if (event !== 'rename') return;
-      // A rename on the watched root's path/basename is the one signal that
-      // survives inode reuse on Linux, where unlinkDir/error never fire.
-      if (this.isRootPath(rawPath) || basename(rawPath) === basename(this.root)) {
-        void this.checkRootLiveness({ force: true });
-      }
+    this.watcher.on('raw', (event: string, rawPath: string | null) => {
+      // libuv reports the root's own move as a rename under the root's name,
+      // and its removal too once nothing holds it open. An entry beneath the
+      // root with that name comes the same way, as does, on macOS, an
+      // FSEvents event for the root that carries no modification flags, such
+      // as its making just before the watch. A held root tells these apart
+      // (checkRootLiveness), so the rename runs the check at once. A root
+      // that isn't held can be made again with the removed one's identity,
+      // so there the rename forces a re-attach. A rename without a name, as
+      // libuv gives for a file whose path it can't read, is passed over.
+      if (event !== 'rename' || typeof rawPath !== 'string') return;
+      if (basename(rawPath) !== basename(this.root)) return;
+      void this.checkRootLiveness({ force: this.rootFd === null });
     });
 
     this.captureRootIdentity();
@@ -204,6 +214,8 @@ export class MountWatcher {
     this.suppressionTimers.clear();
     this.suppressedPaths.clear();
     this.pendingChanges.clear();
+    // Before the close, so a close that fails still lets go of the root.
+    this.releaseRoot();
 
     if (this.watcher) {
       await this.watcher.close();
@@ -299,15 +311,26 @@ export class MountWatcher {
     this.rootPollTimer.unref();
   }
 
+  /**
+   * Whether the root is still the directory the watcher attached to, checked
+   * every `watchRootPollMs` and at a rename under the root's name. A root
+   * replaced at its path, or whose held directory has been removed, is
+   * attached to again, and with `force` so is one that looks unchanged. A
+   * root that's gone is let go of: a read-write mount makes it again and
+   * attaches to it, and a read-only one waits, detached, until a check finds
+   * it back.
+   */
   private async checkRootLiveness(opts: { force?: boolean } = {}): Promise<void> {
     if (this.stopped || this.attaching) return;
 
     let current: RootIdentity;
     try {
       current = this.readRootIdentity();
     } catch (err) {
-      if (this.isErrno(err, 'ENOENT')) {
+      // A held root that has been removed is gone, whatever the path's error.
+      if (this.isErrno(err, 'ENOENT') || this.heldRootRemoved()) {
         this.detached = true;
+        this.releaseRoot();
         if (this.config.mode === 'read-write') {
           await this.reattach();
         }
@@ -327,7 +350,7 @@ export class MountWatcher {
     }
 
     if (!opts.force && !this.detached && this.rootIdentity
-      && this.sameIdentity(current, this.rootIdentity)) {
+      && this.sameIdentity(current, this.rootIdentity) && !this.heldRootRemoved()) {
       return;
     }
 
@@ -341,8 +364,15 @@ export class MountWatcher {
     try {
       const oldWatcher = this.watcher;
       this.watcher = null;
+      this.releaseRoot();
       if (oldWatcher) {
-        await oldWatcher.close();
+        try {
+          await oldWatcher.close();
+        } catch (err) {
+          // chokidar removes its listeners before it closes, so the old
+          // instance reports nothing more, and the root is attached to again.
+          this.reportError(err);
+        }
       }
       if (this.stopped) return;
 
@@ -359,9 +389,11 @@ export class MountWatcher {
 
   private captureRootIdentity(): void {
     try {
-      this.rootIdentity = this.readRootIdentity();
+      this.rootIdentity = this.holdRoot();
       this.detached = false;
     } catch (err) {
+      // What holdRoot opened before it failed isn't held.
+      this.releaseRoot();
       if (this.isErrno(err, 'ENOENT')) {
         this.rootIdentity = null;
         this.detached = true;
@@ -373,14 +405,79 @@ export class MountWatcher {
     }
   }
 
+  /**
+   * The root's identity, with the root held open on Linux until the watcher
+   * lets go of it: at a re-attach, when the root is found gone, or at the
+   * stop. The held directory then says itself whether it has been removed
+   * (heldRootRemoved). A removed directory's inode number can otherwise go
+   * to the next directory made, as ext4 gives it, and one held open isn't
+   * given out. Elsewhere, or where the root can't be opened as a directory,
+   * as without read permission, at the process's file limit or when the
+   * path isn't a directory, it isn't held, and its identity is read from its
+   * path. So too where the root's link count reads 0 as it's opened: if
+   * that's the file system's count for a directory in place, a later check
+   * would take the root for removed and re-attach at every poll.
+   */
+  private holdRoot(): RootIdentity {
+    if (process.platform !== 'linux') return this.readRootIdentity();
+    let fd: number;
+    try {
+      // A directory only, so a FIFO at the path fails at once rather than
+      // blocking until a writer opens it.
+      fd = openSync(this.root, constants.O_RDONLY | constants.O_DIRECTORY);
+    } catch {
+      return this.readRootIdentity();
+    }
+    this.rootFd = fd;
+    const stats = fstatSync(fd);
+    if (stats.nlink === 0) {
+      this.releaseRoot();
+      return this.readRootIdentity();
+    }
+    return identityOf(stats);
+  }
+
+  /**
+   * Whether the directory the watcher holds has been removed, which its link
+   * count says whatever has since been made at its path. On a file system
+   * whose inode numbers a server gives out, the hold can't keep a root made
+   * again from taking the removed one's number, and a removal by another
+   * client shows once this one revalidates the directory's cached
+   * attributes. A held directory that can't be read, as a stale one on a
+   * network file system, counts as removed.
+   */
+  private heldRootRemoved(): boolean {
+    if (this.rootFd === null) return false;
+    try {
+      return fstatSync(this.rootFd).nlink === 0;
+    } catch {
+      return true;
+    }
+  }
+
+  private releaseRoot(): void {
+    if (this.rootFd === null) return;
+    const fd = this.rootFd;
+    this.rootFd = null;
+    try {
+      closeSync(fd);
+    } catch (err) {
+      // Linux frees the descriptor whatever close returns, so the error is
+      // reported and what called this goes on.
+      this.reportError(err);
+    }
+  }
+
   private readRootIdentity(): RootIdentity {
-    const stat = statSync(this.root);
-    return { dev: stat.dev, ino: stat.ino, birthtimeMs: stat.birthtimeMs };
+    return identityOf(statSync(this.root));
   }
 
   private sameIdentity(a: RootIdentity, b: RootIdentity): boolean {
-    // birthtimeMs is only a secondary tripwire: sub-millisecond replacements
-    // can reuse both the inode and timestamp, so raw rename remains essential.
+    // birthtimeMs tells a root made again from a removed one whose inode
+    // number it took, unless both were made within one tick of the clock or
+    // the file system keeps no birth time. A held root's removal shows in its
+    // link count instead, and where the root isn't held the rename's force
+    // covers those.
     return a.dev === b.dev && a.ino === b.ino && a.birthtimeMs === b.birthtimeMs;
   }
 

test/workspace-three-way.test.ts

@@ -1424,7 +1424,8 @@ describe('the catch-up pass at start', () => {
    * re-attach the watcher reports, since a re-attached watcher's ready runs its own pass (here always long before its
    * own bound). A watcher can re-attach just after its first ready. On macOS, by libuv's src/unix/fsevents.c, an
    * FSEvents event for the watched directory itself, when it carries no modification flags, reaches fs.watch as a
-   * rename under the directory's own name, and the watcher's raw handler re-attaches on any rename named like its root.
+   * rename under the directory's own name, and there, holding no root, the watcher re-attaches at any rename named
+   * like its root.
    */
   const assertPassesOnceReady = (env: Env, passes: number, expected: number) => {
     const reattaches = env.events.filter((e) => e.type === 'workspace:watcher-reattached').length;
@@ -1452,17 +1453,19 @@ describe('the catch-up pass at start', () => {
   test('a re-attached watcher runs its pass at its own ready, so a change after that pass reaches the store', { skip: process.platform !== 'linux' }, async (t) => {
     const env = new Env(t);
     env.writeDisk('gate.json', 'v1');
-    // The root poll is past the test's deadline, so only the root's rename event re-attaches the watcher.
+    // The root poll is past the test's deadline, so only the re-attach below happens.
     const m = await env.open({ watch: 'always', watchDebounceMs: 50, watchRootPollMs: 60_000 });
     const mount = (m as any).mounts.get('work');
     await until(() => !!mount.watcherReadyAt && storedText(env, 'gate.json') === 'v1', 'the first pass took gate.json in');
-    // The root is moved away, its file changed, and moved back, so the watcher re-attaches to the same directory,
-    // which the module still agrees with. chokidar's directory reads each wait a second meanwhile, so the
-    // re-attached watcher's scan reaches gate.json long after a pass started at the re-attach would have run.
+    // The watcher re-attaches to the same directory, which the module still agrees with, as it does at a rename
+    // named like a root it can't hold open. gate.json changes once the old chokidar instance has begun to close, which
+    // takes its listeners first, so no watcher reports the change. chokidar's directory reads each wait a second
+    // meanwhile, so the re-attached watcher's scan reaches gate.json long after a pass started at the re-attach would
+    // have run.
     await withChokidarReaddir(() => new Promise((r) => setTimeout(r, 1000)), async (reads) => {
-      renameSync(env.dir, `${env.dir}.away`);
-      writeFileSync(join(`${env.dir}.away`, 'gate.json'), 'v2');
-      renameSync(`${env.dir}.away`, env.dir);
+      const reattaching: Promise<void> = (m as any).watchers.get('work').reattach();
+      env.writeDisk('gate.json', 'v2');
+      await reattaching;
       await until(() => reattached(env), 'the watcher re-attached');
       await until(() => storedText(env, 'gate.json') === 'v2', 'gate.json\'s v2 was taken in', 15_000);
       env.writeDisk('gate.json', 'v3');
@@ -1529,8 +1532,9 @@ describe('the catch-up pass at start', () => {
     const m = await env.open({ watch: 'always', watchDebounceMs: 50 }, {}, (m) => { counted = countPasses(m); });
     const mount = (m as any).mounts.get('work');
     await until(() => !!mount.watcherReadyAt && storedText(env, 'gate.json') === 'v1', 'the pass ran at ready');
-    // The rename macOS reports for an FSEvents event on the root itself, sent to the watcher's chokidar instance.
-    (m as any).watchers.get('work').watcher.emit('raw', 'rename', 'work', {});
+    // The watcher re-attaches, as it does on macOS, where it holds no root, at the rename an FSEvents event on the root
+    // itself arrives as.
+    await (m as any).watchers.get('work').reattach();
     await until(() => counted.passes >= 2, "the re-attached watcher's ready ran its pass");
     await elapse(200);
     assert.ok(reattached(env), 'the watcher re-attached');