fix(metadata-fs): register written paths with the watcher instead of trusting its scan (#7282)

.changeset/metadata-fs-watch-write-registration.md

@@ -0,0 +1,41 @@
+---
+"@objectstack/metadata-fs": patch
+---
+
+fix(metadata-fs): register every written path with the watcher, so an item created while chokidar is still scanning is not invisible forever (#7282)
+
+`FileSystemRepository`'s watcher could go **permanently blind to a single
+item** — external edits to that file produced no `MetadataEvent` for the whole
+life of the process, and nothing recovered short of a restart. The window is a
+race between chokidar's asynchronous initial scan and the repository's own
+first write, and both `start()` (which arms the watcher, after which the caller
+may `put()` on the next tick) and `ensureRoot()` (which arms it in the middle
+of the very first write, #7000) can open it.
+
+Measured on chokidar 5 with this repository's options (`usePolling`,
+`interval: 1000`):
+
+1. chokidar reads `<root>/<type>/` and finds it EMPTY — the atomic `rename` in
+   `writeJsonAtomic` has not landed yet;
+2. the rename lands, changing the directory's mtime;
+3. chokidar calls `watchFile()` on that directory and libuv takes its polling
+   baseline stat, which already reflects step 2.
+
+The directory's stat then never changes again, so no poll ever fires for it,
+the directory is never re-read, the item file is never added to the watched
+set, and no per-file watcher is created. `getWatched()` reports the type
+directory as `[]` while the file sits in it, and neither `add` nor `change` is
+ever emitted for that path.
+
+The fix does not widen any timer. The only writer that can be inside that
+window is the repository itself, so `put()` now tells the watcher explicitly
+about the path it created instead of depending on a directory scan that may
+never notice it. Registration is idempotent and emits nothing.
+
+User-visible effect: `MetadataManager.subscribe()` (and every consumer of
+`repo.watch()`) now reliably sees out-of-process edits — a hand edit, or a
+`git checkout` bringing metadata JSON in — to items written earlier in the same
+process. This was also the cause of four merge-queue ejections across three
+PRs; the two time-based mitigations tried before it (a 20s/25s event deadline
+and a wider pre-edit sleep) could not have worked, because the event was never
+delivered rather than late.

packages/metadata-fs/src/repository.ts

@@ -305,6 +305,9 @@ export class FileSystemRepository implements MetadataRepository {
         // we keep it in selfWrites for one debounce tick.
         setTimeout(() => this.selfWrites.delete(file), 200);
       }
+      // The watcher must not depend on its own directory scan to notice a
+      // path we created ourselves (#7282). See `trackWrittenPath`.
+      this.trackWrittenPath(file);
       this.heads.set(key, hash);
 
       const evt: MetadataEvent = {
@@ -433,6 +436,53 @@ export class FileSystemRepository implements MetadataRepository {
     return last;
   }
 
+  /**
+   * Register a path this repository just wrote with the watcher (#7282).
+   *
+   * chokidar's initial scan is asynchronous, and every write path here can be
+   * running **while it is still walking the tree** — `start()` arms the watcher
+   * and the caller may `put()` on the next tick, and `ensureRoot()` arms it in
+   * the middle of the very first write. With `usePolling` that combination has
+   * a permanently-blinding interleaving, measured on chokidar 5 with this
+   * repository's own options:
+   *
+   *   1. chokidar reads `<root>/<type>/` and finds it EMPTY — the atomic
+   *      `rename` in `writeJsonAtomic` has not landed yet.
+   *   2. the rename lands; the directory's mtime changes.
+   *   3. chokidar calls `watchFile()` on that directory, and libuv takes its
+   *      polling baseline stat — which already reflects step 2.
+   *
+   * From then on the directory's stat never changes again, so no poll ever
+   * fires for it, `_handleRead` never re-runs, the item file is never added to
+   * the watched set, and no per-file watcher is ever created. chokidar emits
+   * neither `add` nor `change` for that path **for the life of the process** —
+   * `getWatched()` reports the type directory as `[]` forever while the file
+   * sits in it. That is the whole of #7282: the four merge-queue ejections all
+   * waited out their deadlines (20s, then 25541ms against 25s) on an event that
+   * was never going to be delivered, which is why widening the deadline and
+   * widening the pre-edit sleep both changed nothing, and why lowering
+   * `interval` would change nothing either — a shorter poll re-compares against
+   * the same unchanged directory stat.
+   *
+   * The window is exactly "files that exist at baseline time but were absent
+   * from the snapshot read a moment earlier", and the only writer that can be
+   * inside it is us. So we close it at the source: tell the watcher explicitly
+   * about every path we create, instead of hoping its scan happened to see it.
+   *
+   * `add()` is idempotent here — `_handleFile` returns early when the parent
+   * directory already tracks the basename — and it emits nothing, because
+   * chokidar treats an explicit `add()` as an initial add and `ignoreInitial`
+   * is set. Its effect is the one we need: `_watchWithNodeFs` registers the
+   * basename with the parent directory (without which chokidar drops `change`
+   * events for the file) and starts the per-file poll.
+   */
+  private trackWrittenPath(file: string): void {
+    const w = this.watcher;
+    // `add()` clears `closed`, so never hand a closing watcher a new path.
+    if (!w || w.closed) return;
+    w.add(file);
+  }
+
   private startWatcher(): void {
     const root = this.layout.root;
     const w = chokidar.watch(root, {

packages/metadata-fs/test/watch-write-registration.test.ts

@@ -0,0 +1,183 @@
+// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
+
+/**
+ * #7282 — a path this repository writes must be registered with the watcher
+ * **without waiting for a poll interval**.
+ *
+ * The defect this pins is a permanently-blinding interleaving between
+ * chokidar's asynchronous initial scan and our own first write, measured on
+ * chokidar 5 with this repository's options (`usePolling`, `interval: 1000`):
+ *
+ *   1. chokidar reads `<root>/<type>/` and finds it EMPTY — `writeJsonAtomic`'s
+ *      `rename` has not landed yet.
+ *   2. the rename lands; the directory's mtime changes.
+ *   3. chokidar calls `watchFile()` on the directory and libuv takes its
+ *      polling baseline stat, which already reflects step 2.
+ *
+ * The directory's stat then never changes again, so the directory is never
+ * re-read, the item file is never registered, no per-file watcher is created,
+ * and neither `add` nor `change` is emitted for it **for the life of the
+ * process**. Measured directly: `getWatched()` reported the type directory as
+ * `[]` for the full 7.5s of a probe run while the file sat in it, and
+ * `handleFsChange` was never entered once.
+ *
+ * That is why both time-based mitigations tried on this family failed: the
+ * event is never delivered, so a 20s deadline (#7208) and a 25541ms one (#7255)
+ * both ran out with an empty array, and a wider pre-edit sleep cannot help
+ * either — the damage is done before the sleep starts. Lowering `interval`
+ * cannot help for the same reason: a shorter poll re-compares against the same
+ * unchanged directory stat.
+ *
+ * ## Why this file asserts registration rather than the race
+ *
+ * The interleaving above lives inside chokidar, between a `readdirp` stream end
+ * and a `watchFile()` call, so it cannot be forced from the outside without a
+ * test seam. What CAN be pinned deterministically is the property the fix
+ * establishes, which is what actually closes the window: after `put()` returns,
+ * the watcher knows the path **from us**, not from a directory scan that may
+ * never happen.
+ *
+ * The two outcomes are separated structurally, not by a lucky margin:
+ *
+ *   - with the fix, registration costs one `stat` (single-digit ms);
+ *   - without it, the earliest the watcher can learn the path is its next
+ *     directory poll — a full `interval` (1000ms) plus the `awaitWriteFinish`
+ *     stability window.
+ *
+ * The case anchors the poll phase before measuring so that "one poll away" is
+ * genuinely a full interval away: it lets an out-of-process file land first and
+ * waits for the repository to report it, which happens only when a poll tick
+ * for the type directory has just fired. `REGISTRATION_BUDGET_MS` then sits an
+ * order of magnitude below the remaining interval.
+ */
+
+import { describe, it, expect, beforeEach, afterEach } from 'vitest';
+import fs from 'node:fs/promises';
+import path from 'node:path';
+import os from 'node:os';
+import type { MetaRef, MetadataEvent } from '@objectstack/metadata-core';
+import { FileSystemRepository } from '../src/index.js';
+
+const ref = (name: string): MetaRef => ({ org: 'system', type: 'view', name });
+const sleep = (ms: number) => new Promise<void>((r) => setTimeout(r, ms));
+
+/**
+ * How long registration may take after `put()` resolves. Comfortably above one
+ * `stat` under a saturated runner and comfortably below the watcher's 1000ms
+ * poll interval, which is the only other way the path could get registered.
+ */
+const REGISTRATION_BUDGET_MS = 400;
+
+/** Deadline for the positive waits that anchor the poll phase and control the case. */
+const EVENT_WAIT_MS = 20_000;
+
+const CASE_TIMEOUT_MS = 60_000;
+
+/**
+ * chokidar's watched-path registry, reached through the repository's private
+ * watcher handle. `getWatched()` is chokidar's own public API; only the handle
+ * is internal, and no public surface reports it.
+ */
+interface WatcherHandle {
+  watcher: {
+    getWatched(): Record<string, string[]>;
+    once(event: 'ready', listener: () => void): unknown;
+  } | null;
+}
+
+const handleOf = (repo: FileSystemRepository) => {
+  const w = (repo as unknown as WatcherHandle).watcher;
+  if (!w) throw new Error('watcher not armed — the case cannot measure anything');
+  return w;
+};
+
+const watchedIn = (repo: FileSystemRepository, dir: string): string[] =>
+  handleOf(repo).getWatched()[dir] ?? [];
+
+describe('FileSystemRepository watcher — writes register their own path (#7282)', () => {
+  let root: string;
+  let repo: FileSystemRepository | undefined;
+
+  beforeEach(async () => {
+    root = await fs.mkdtemp(path.join(os.tmpdir(), 'objectstack-fs7282-'));
+  });
+
+  afterEach(async () => {
+    if (repo) await repo.close().catch(() => undefined);
+    repo = undefined;
+    await fs.rm(root, { recursive: true, force: true });
+  });
+
+  it('registers a put() path with the watcher without waiting for a poll', async () => {
+    const viewDir = path.join(root, 'view');
+    // The type directory exists and is non-empty before the watcher arms, so
+    // chokidar's scan cannot be blamed for anything measured below.
+    await fs.mkdir(viewDir, { recursive: true });
+    await fs.writeFile(path.join(viewDir, 'seed.json'), JSON.stringify({ label: 'seed' }, null, 2));
+
+    repo = new FileSystemRepository({ root, org: 'system' }); // watcher ENABLED
+    await repo.start();
+    // Attach before yielding. `start()` arms the watcher as its last statement
+    // and chokidar cannot finish its walk without at least one async stat, so
+    // `ready` provably has not fired yet. Waiting for it matters: a file that
+    // lands while the initial walk is still running is treated as pre-existing
+    // and, under `ignoreInitial`, emits nothing at all — measured 3/3. The
+    // anchor below must not be subject to that.
+    const scanned = new Promise<void>((res) => { handleOf(repo!).once('ready', res); });
+
+    const iter = repo.watch({ org: 'system' }, 999)[Symbol.asyncIterator]();
+    const events: MetadataEvent[] = [];
+    let resolveNext: (() => void) | null = null;
+    void (async () => {
+      for (;;) {
+        const next = await iter.next();
+        if (next.done) return;
+        events.push(next.value as MetadataEvent);
+        resolveNext?.();
+      }
+    })();
+    const nextEvent = () => new Promise<void>((res) => { resolveNext = res; });
+
+    await Promise.race([scanned, sleep(EVENT_WAIT_MS)]);
+    // The walk reached the type directory, so what follows is measuring the
+    // steady state and not the initial scan.
+    expect(watchedIn(repo, viewDir)).toContain('seed.json');
+
+    // ── Anchor the poll phase ────────────────────────────────────────────
+    // An out-of-process file lands in the type directory; the repository can
+    // only report it once a poll tick for that directory has fired. Returning
+    // from this wait therefore means we are at the START of a fresh interval,
+    // so "one poll away" below really is ~1000ms away and not ~5ms away.
+    const anchored = nextEvent();
+    await fs.writeFile(path.join(viewDir, 'anchor.json'), JSON.stringify({ label: 'anchor' }, null, 2));
+    await Promise.race([anchored, sleep(EVENT_WAIT_MS)]);
+    expect(events.map((e) => e.ref.name)).toContain('anchor');
+
+    // ── The measurement ──────────────────────────────────────────────────
+    await repo.put(ref('fresh'), { label: 'fresh' }, { parentVersion: null, actor: 'tester' });
+
+    const deadline = Date.now() + REGISTRATION_BUDGET_MS;
+    while (!watchedIn(repo, viewDir).includes('fresh.json') && Date.now() < deadline) {
+      await sleep(10);
+    }
+    expect(watchedIn(repo, viewDir)).toContain('fresh.json');
+
+    // ── Liveness control ─────────────────────────────────────────────────
+    // ⚠️ Green in BOTH directions on a quiet machine — the registration race is
+    // load-dependent, so this cannot be the case's evidence. It is here to
+    // prove the registration above is the real thing and not bookkeeping: an
+    // external edit to the freshly written path must still reach subscribers.
+    const edited = nextEvent();
+    await fs.writeFile(
+      path.join(viewDir, 'fresh.json'),
+      JSON.stringify({ label: 'fresh, edited on disk' }, null, 2),
+    );
+    await Promise.race([edited, sleep(EVENT_WAIT_MS)]);
+    await iter.return?.(undefined);
+
+    const updates = events.filter((e) => e.ref.name === 'fresh' && e.op === 'update');
+    expect(updates).toHaveLength(1);
+    expect(updates[0]!.source).toBe('fs');
+    expect(updates[0]!.actor).toBe('fs');
+  }, CASE_TIMEOUT_MS);
+});