fix(metadata-fs): confirm absence on disk before publishing an external delete (#12695)

A watcher unlink is a claim of absence, not absence. chokidar reaches its
removal path from failed stats as well as from real removals, so under
filesystem pressure it retires files that are still there; publishing those
claims produced a durable delete/create pair for an item nobody removed.

Confirm against the disk under the same per-key lock the reconciliation sweep
already used for this, and fall through to the content path when the file is
still present, so a spurious unlink alongside a real external edit surfaces as
the update it always was.

Part of #7369


Claude-Session: https://claude.ai/code/session_01DKWDdUJ2XNRESVVWUvcpnh

Co-authored-by: Claude <jack@objectstack.ai>
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>

Author: 3 people

.changeset/watcher-unlink-requires-absence.md

@@ -0,0 +1,9 @@
+---
+'@objectstack/metadata-fs': patch
+---
+
+The file watcher no longer publishes a `delete` for an item that is still on disk.
+
+A watcher `unlink` is a claim of absence, not absence: chokidar reaches its removal path from failed stats as well as from real removals, so under filesystem pressure it can retire a file that is still there. `FileSystemRepository` published those claims straight through as `delete` events — appended to the change log and broadcast to every subscriber, which drops the item from the metadata registry and the `list()` cache — and the reconciliation sweep then republished the untouched file as a `create`. A failed stat therefore produced a durable delete/create pair for an item nobody removed, with a window in between where live metadata had disappeared.
+
+The removal face now confirms the absence against the disk under the same per-key lock the reconciliation sweep already used for this, and an `unlink` for a path that still exists falls through to the content comparison — so a spurious unlink that arrived alongside a real external edit surfaces as the `update` it always was. Genuine external removals are unaffected and are still published on the first delivery.

packages/metadata-fs/src/repository.ts

@@ -901,9 +901,36 @@ export class FileSystemRepository implements MetadataRepository {
    *   - `unlink` — `!currentHead` drops the event when the index already
    *     agrees the item is gone. `delete()` retires the head *before* it
    *     unlinks, precisely because this face gets no `awaitWriteFinish` delay.
+   *     This is the whole of the *self-write* answer on this face, and it is
+   *     not the whole of the face — see the section below it.
    *
    * Both faces are pinned together in `test/self-write-suppression.test.ts`.
    *
+   * ## A removal is confirmed against the disk before it is published (#7369)
+   *
+   * Those two checks answer "is this event OURS". Neither answers "did this
+   * happen at all", and the `unlink` face needs that second question asked
+   * because its input is a third party's inference: chokidar decides a file is
+   * gone from a *stat that failed*, not only from a file that went away, and
+   * `!currentHead` cannot tell the two apart because a spurious unlink leaves
+   * the index exactly as valid as it was.
+   *
+   * The cost of getting it wrong is not a dropped notification, which the
+   * sweep would repair. A `delete` is appended to the change log and broadcast
+   * to every subscriber, and `MetadataManager` drops the item from the
+   * registry and the `list()` cache on receipt. The sweep then finds the file
+   * still on disk and republishes it as a `create` — so a failed stat becomes
+   * a durable, permanently recorded delete/create pair for an item that never
+   * changed, and every consumer sees the item disappear in between. That is
+   * the shape ADR-0008's log is least able to walk back.
+   *
+   * So `existsSync` under the same per-key lock the sweep uses, and the same
+   * decision it makes: absent ⇒ publish the removal; present ⇒ this was a
+   * change, answered by the content path below. Genuine removals pay nothing —
+   * `delete()` is still suppressed by `!currentHead`, and an external `rm` is
+   * still published on the first delivery, because for those the file really
+   * is gone. Pinned in `test/external-delete-requires-absence.test.ts`.
+   *
    * Note the deliberate limit: identity is judged on what round-trips through
    * the file, so a spec whose in-memory form does not (a `Date`, which
    * canonicalises to `{}` in memory but to an ISO string once written and
@@ -922,7 +949,36 @@ export class FileSystemRepository implements MetadataRepository {
     };
     const key = refKey(ref);
     await this.mutex.run(key, async () => {
-      if (kind === 'unlink') {
+      // A watcher `unlink` is a CLAIM of absence, not absence — so it is
+      // confirmed against the disk before a `delete` is published (#7369).
+      // `!currentHead` inside `publishExternalDelete` cannot do this job: it
+      // compares against the INDEX, which is exactly what a spurious unlink
+      // leaves intact. The reconciliation sweep already re-checks disk truth
+      // under this same lock before retiring a key, for a reason it states in
+      // place; the watcher face was the one path that published a removal on
+      // the observer's word alone.
+      //
+      // chokidar reaches its removal path from failed *stats* as well as from
+      // real removals, and says so at both sites (chokidar 5 `handler.js`):
+      // `_handleFile`'s poll listener re-stats a file whose watched stat came
+      // back zeroed and calls `_remove` from the catch — under the comment
+      // "Fix issues where mtime is null but file is still present" — with no
+      // discrimination on errno, so EMFILE/ENFILE retires a file that is
+      // there; and `_handleRead`'s snapshot diff `_remove`s every previously
+      // tracked entry its readdirp pass did not enumerate, which includes the
+      // entries whose per-entry `lstat` failed rather than only the ones that
+      // are gone. Both faults are load-shaped, which is why the merge queue —
+      // the only context that runs the FULL suite — is where this surfaced,
+      // twice, on a case that touches nothing else.
+      //
+      // Falling through is the repair, not just skipping: when the path is
+      // still there the honest reading of the event is "something happened to
+      // this file", which is the content path's question. It answers with the
+      // same `currentHead === hash` comparison used everywhere else, so a
+      // spurious unlink that accompanied a real in-place edit still surfaces
+      // as the `update` it always was, in the same tick, rather than as the
+      // `delete` + `create` pair the index-only check produced.
+      if (kind === 'unlink' && !existsSync(absPath)) {
         await this.publishExternalDelete(ref, key);
         return;
       }

packages/metadata-fs/test/external-delete-requires-absence.test.ts

@@ -0,0 +1,245 @@
+// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
+
+/**
+ * #7369 — a watcher `unlink` is a CLAIM of absence. Only the disk settles it.
+ *
+ * ## The defect
+ *
+ * `handleFsChange`'s removal face used to publish on the observer's word:
+ *
+ *   if (kind === 'unlink') { await this.publishExternalDelete(ref, key); return; }
+ *
+ * and the only suppression inside `publishExternalDelete` is `!currentHead` —
+ * a comparison against the **index**, which a spurious unlink leaves exactly
+ * as valid as it was. So an event that merely *claimed* a file was gone became
+ * a `delete`: appended to the change log, broadcast to every subscriber, and
+ * acted on by `MetadataManager`, which drops the item from the registry and
+ * the `list()` cache on receipt. The reconciliation sweep then found the file
+ * still on disk and republished it as a `create`. A failed stat therefore
+ * produced a durable delete/create pair for an item that was never removed,
+ * with a window in between where live metadata had vanished.
+ *
+ * The repository already held the opposite discipline one method away: the
+ * sweep's delete pass re-checks `existsSync` under the per-key lock before
+ * retiring a key, and `external-write-resync.test.ts` pins that a false
+ * absence handed to the sweep must produce **zero** `delete` events. The
+ * watcher face was the one path that skipped the check.
+ *
+ * ## Why chokidar's unlink is not evidence of removal
+ *
+ * chokidar reaches its removal path from failed **stats** as well as from real
+ * removals, and both sites are load-shaped (chokidar 5, `handler.js`):
+ *
+ *   - `_handleFile`'s poll listener runs when `fs.watchFile` reports a zeroed
+ *     stat, re-stats the file, and calls `_remove` from the `catch` — under
+ *     the comment "Fix issues where mtime is null but file is still present",
+ *     with no discrimination on errno. EMFILE/ENFILE retires a file that is
+ *     there.
+ *   - `_handleRead`'s snapshot diff `_remove`s every previously tracked entry
+ *     that its readdirp pass did not enumerate — which includes entries whose
+ *     per-entry `lstat` failed, not only entries that are gone.
+ *
+ * That is why this surfaced in the merge queue and nowhere else: the queue is
+ * the only context that runs the FULL suite, and `watch-dot-root.test.ts` case
+ * 1 has been ejected from it twice by a delivery fault it did not cause. The
+ * second ejection (2026-08-27, run 33057527457, `Test Core (2/6)`) failed with
+ * `expected 'delete' to be 'update'` at `watch-dot-root.test.ts:268` — the
+ * exact-count assertion on the line above it PASSED, so exactly one event
+ * arrived, typed `delete`, for a file the case never removes.
+ *
+ * ## Why these cases assert at the handler seam
+ *
+ * The mechanism upstream is a failed stat under resource pressure, which
+ * cannot be summoned on demand and would be a wall-clock race to wait for.
+ * This package has now been ejected from the merge queue three times by
+ * wall-clock watcher assertions, so the cases follow the discipline
+ * `self-write-suppression.test.ts` set for the same reason: enter at the seam
+ * immediately below chokidar, with exactly the arguments a spurious unlink
+ * delivers, and assert the contract rather than the race.
+ *
+ * The contract, stated once:
+ *
+ *   **a removal is published only for an item that is actually absent from
+ *   disk; an `unlink` for a path that is still there is a change.**
+ *
+ * Both directions are asserted, because each alone has a trivial wrong fix: a
+ * repository that published nothing on `unlink` would pass the first two cases
+ * and fail the third, and today's code passes the third and fails the first
+ * two.
+ */
+
+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));
+
+/**
+ * Enter the watcher handler exactly as chokidar's `unlink` listener does —
+ * `w.on('unlink', (p) => void this.handleFsChange(p, 'unlink'))` in
+ * `startWatcher`. The same reach is made by `self-write-suppression.test.ts`.
+ */
+type Kind = 'add' | 'change' | 'unlink';
+const deliver = (repo: FileSystemRepository, file: string, kind: Kind): Promise<void> =>
+  (repo as unknown as {
+    handleFsChange(p: string, k: Kind): Promise<void>;
+  }).handleFsChange(file, kind);
+
+/**
+ * Enough turns for anything the delivery above would publish to reach the
+ * subscriber. It is not a race budget: `deliver` is awaited, so the publish has
+ * already happened or is never going to — this only lets the broker's queue
+ * drain into the array.
+ */
+const drain = () => sleep(50);
+
+describe('#7369 an external delete is published only for an item that is really gone', () => {
+  let root: string;
+  let repo: FileSystemRepository | null = null;
+  let events: MetadataEvent[] = [];
+  let stop: (() => Promise<void>) | null = null;
+
+  /**
+   * The watcher is disabled throughout: these cases supply the event
+   * themselves, and a live poller would race them. It also retires the
+   * reconciliation sweep (armed inside `startWatcher`), so every event
+   * observed here came from the delivery the case made — which is the whole
+   * point, since the sweep is precisely what used to paper over the defect by
+   * republishing the item as a `create` two seconds later.
+   */
+  async function start(): Promise<void> {
+    repo = new FileSystemRepository({ root, org: 'system', disableWatch: true });
+    await repo.start();
+    const iter = repo.watch({ org: 'system' }, 999)[Symbol.asyncIterator]();
+    let stopped = false;
+    void (async () => {
+      while (!stopped) {
+        const next = await iter.next();
+        if (next.done) return;
+        events.push(next.value as MetadataEvent);
+      }
+    })();
+    stop = async () => {
+      stopped = true;
+      await iter.return?.(undefined);
+    };
+  }
+
+  beforeEach(async () => {
+    root = await fs.mkdtemp(path.join(os.tmpdir(), 'os-unlink-claim-'));
+    events = [];
+  });
+
+  afterEach(async () => {
+    if (stop) await stop();
+    stop = null;
+    if (repo) await repo.close();
+    repo = null;
+    await fs.rm(root, { recursive: true, force: true });
+  });
+
+  it('an unlink for a path that still exists, edited externally, is the update it always was', async () => {
+    await start();
+    const r = ref('case_grid');
+    const file = path.join(root, 'view', 'case_grid.json');
+    const first = await repo!.put(r, { label: 'original' }, {
+      parentVersion: null,
+      actor: 'tester',
+    });
+    // Let `put()`'s own event land before clearing, so it cannot arrive
+    // afterwards and be read as something the delivery below produced.
+    await drain();
+    events.length = 0;
+
+    // The queue failure, reproduced at the seam: an external in-place edit
+    // lands, and the delivery chokidar makes for it is `unlink`.
+    await fs.writeFile(file, JSON.stringify({ label: 'externally edited' }, null, 2) + '\n');
+    await deliver(repo!, file, 'unlink');
+    await drain();
+
+    expect(events).toHaveLength(1);
+    // Pre-fix this was 'delete'. That is the assertion that ejected two
+    // unrelated PRs from the merge queue.
+    expect(events[0]!.op).toBe('update');
+    expect(events[0]!.ref.name).toBe('case_grid');
+    expect(events[0]!.source).toBe('fs');
+    expect(events[0]!.actor).toBe('fs');
+    expect(events[0]!.parentHash).toBe(first.version);
+    expect(events[0]!.hash).not.toBeNull();
+
+    // And the item is still there — the index was never retired behind the
+    // subscriber's back.
+    const item = await repo!.get(r);
+    expect(item).not.toBeNull();
+    expect(item!.body).toEqual({ label: 'externally edited' });
+  });
+
+  it('a purely spurious unlink — nothing on disk changed — publishes nothing at all', async () => {
+    await start();
+    const r = ref('untouched');
+    const file = path.join(root, 'view', 'untouched.json');
+    const first = await repo!.put(r, { label: 'stable' }, {
+      parentVersion: null,
+      actor: 'tester',
+    });
+    // Let `put()`'s own event land before clearing, so it cannot arrive
+    // afterwards and be read as something the delivery below produced.
+    await drain();
+    events.length = 0;
+
+    // The pure fault: chokidar's stat failed, the file never moved.
+    await deliver(repo!, file, 'unlink');
+    await drain();
+
+    // Not "a delete then a create". Nothing happened, so nothing is published,
+    // and no phantom pair is written to the change log for a consumer to
+    // replay forever.
+    expect(events).toEqual([]);
+    const item = await repo!.get(r);
+    expect(item).not.toBeNull();
+    expect(item!.hash).toBe(first.version);
+
+    // The change log must be equally clean: `delete` is durable, and the
+    // history is what a consumer rebuilding from disk reads.
+    const history: MetadataEvent[] = [];
+    for await (const evt of repo!.history(r, {})) history.push(evt);
+    expect(history.map((h) => h.op)).toEqual(['create']);
+  });
+
+  it('a genuine external removal is still published on the first delivery', async () => {
+    await start();
+    const r = ref('really_gone');
+    const file = path.join(root, 'view', 'really_gone.json');
+    const first = await repo!.put(r, { label: 'here' }, {
+      parentVersion: null,
+      actor: 'tester',
+    });
+    // Let `put()`'s own event land before clearing, so it cannot arrive
+    // afterwards and be read as something the delivery below produced.
+    await drain();
+    events.length = 0;
+
+    // Somebody ran `rm`. The file is absent, so the claim is true.
+    await fs.rm(file);
+    await deliver(repo!, file, 'unlink');
+    await drain();
+
+    // ⚠️ The complementary direction, and the reason the fix is a disk check
+    // rather than "ignore unlink": without this case, dropping every unlink
+    // would pass the two above. External removals are a real capability —
+    // `external-write-resync.test.ts` pins the sweep's recovery of one.
+    expect(events).toHaveLength(1);
+    expect(events[0]!.op).toBe('delete');
+    expect(events[0]!.ref.name).toBe('really_gone');
+    expect(events[0]!.hash).toBeNull();
+    expect(events[0]!.parentHash).toBe(first.version);
+    expect(events[0]!.source).toBe('fs');
+    expect(events[0]!.actor).toBe('fs');
+
+    expect(await repo!.get(r)).toBeNull();
+  });
+});