Prevent stale 404s for files written by another realm-server instance

packages/realm-server/lib/realm-file-changes-listener.ts

@@ -13,14 +13,21 @@ const log = logger('realm-server:file-changes-listener');
 // runtime-common/realm.ts), every listener subscribed on this channel looks
 // up the URL in its lookup function. If the realm is mounted locally,
 // `realm.invalidateCache(path)` clears the matching #sourceCache /
-// #transpiledModuleCache entries. If it's not mounted, the notification is dropped —
-// this instance has no stale state to clear.
+// #transpiledModuleCache entries, and `realm.refreshDirectoryView(path)`
+// re-lists the path's parent directory so the kernel's cached view of the
+// shared filesystem (which can still say the file is absent) is refreshed
+// too. If it's not mounted, the notification is dropped — this instance has
+// no stale state to clear.
 //
 // Bulk variant: when the path is the wildcard sentinel `*` (CS-11156),
 // `realm.clearLocalSourceCaches()` drops every cached path for that realm. Emitted
 // by the publish-realm / unpublish-realm / delete-realm handlers after the
 // FS swap or removal so peers (whose file-watcher events do NOT cross
-// replicas) bypass their pre-swap cached bytes on the next read.
+// replicas) bypass their pre-swap cached bytes on the next read. Note the
+// wildcard branch clears only the in-memory byte caches: the kernel's view of
+// the swapped directory tree is NOT refreshed here (that would take a
+// recursive walk of the realm), so a peer can still serve stale
+// file-not-found for files a republish added, until the kernel cache expires.
 //
 // The LISTEN is backed by `PgAdapter.subscribe` (shared multiplexed
 // notification client). There is no periodic work to run between
@@ -102,6 +109,13 @@ export class RealmFileChangesListener {
         realm.clearLocalSourceCaches();
       } else {
         realm.invalidateCache(parsed.path);
+        // Fire-and-forget: the NOTIFY handler stays synchronous, and a failed
+        // directory listing only means the kernel cache expires on its own.
+        realm.refreshDirectoryView(parsed.path).catch((err: unknown) => {
+          log.warn(
+            `refreshDirectoryView failed for ${parsed.url} ${parsed.path}: ${String(err)}`,
+          );
+        });
       }
     } catch (err: unknown) {
       const op = isWildcard ? 'clearLocalSourceCaches' : 'invalidateCache';

packages/realm-server/node-realm.ts

@@ -18,10 +18,9 @@ import type { LocalPath } from '@cardstack/runtime-common/paths';
 import type { ServerResponse } from 'http';
 import sane, { type Watcher } from 'sane';
 
-import type { ReadStream, Stats } from 'fs-extra';
+import type { Dirent, ReadStream, Stats } from 'fs-extra';
 import fsExtra from 'fs-extra';
 const {
-  readdirSync,
   existsSync,
   writeFileSync,
   statSync,
@@ -61,26 +60,6 @@ function statIfExists(absolutePath: string): Stats | undefined {
   }
 }
 
-// The same vanishing-path race applies to a directory listing: a concurrent
-// delete — e.g. a published realm being unpublished while a mtimes traversal is
-// descending its tree — can remove a directory between a parent listing that
-// yielded it and the recursive read that opens it, so treat a vanished
-// directory as empty rather than letting the raw ENOENT escape. Only ENOENT is
-// swallowed: unlike statIfExists (whose callers probe arbitrary paths), the
-// traversal only ever reads directories it just listed, and ENOTDIR — the
-// target is a regular file — is a genuine not-a-directory that callers such as
-// directoryEntries must still see.
-function readdirIfExists(absolutePath: string) {
-  try {
-    return readdirSync(absolutePath, { withFileTypes: true });
-  } catch (err: any) {
-    if (err?.code === 'ENOENT') {
-      return undefined;
-    }
-    throw err;
-  }
-}
-
 function parseMatrixSendEventError(error: unknown): {
   status?: number;
   errcode?: string;
@@ -140,11 +119,20 @@ export class NodeAdapter implements RealmAdapter {
       ensureDirSync(path);
     }
     let absolutePath = join(this.realmDir, path);
-    let entries = readdirIfExists(absolutePath);
-    if (!entries) {
-      return;
+    // Asynchronous on purpose: on a network filesystem a listing is a round
+    // trip, and this generator runs on the request path and in the
+    // cross-instance directory refresh, neither of which may block the event
+    // loop. A missing directory lists as empty.
+    let entries: Dirent[];
+    try {
+      entries = await fsExtra.readdir(absolutePath, { withFileTypes: true });
+    } catch (err: any) {
+      if (err?.code === 'ENOENT') {
+        return;
+      }
+      throw err;
     }
-    for await (let entry of entries) {
+    for (let entry of entries) {
       let isDirectory = entry.isDirectory();
       let isFile = entry.isFile();
       if (!isDirectory && !isFile) {

packages/realm-server/tests/directory-view-refresher-test.ts

@@ -0,0 +1,162 @@
+import QUnit from 'qunit';
+const { module, test } = QUnit;
+import { basename } from 'path';
+import {
+  DirectoryViewRefresher,
+  ancestorDirectories,
+} from '@cardstack/runtime-common';
+
+module(basename(import.meta.filename), function () {
+  test('ancestorDirectories lists the realm root first, then each directory down to the parent', function (assert) {
+    assert.deepEqual(ancestorDirectories('c.json'), ['']);
+    assert.deepEqual(ancestorDirectories('a/c.json'), ['', 'a']);
+    assert.deepEqual(ancestorDirectories('a/b/c.json'), ['', 'a', 'a/b']);
+  });
+
+  test('refresh lists every ancestor, root first', async function (assert) {
+    let listed: string[] = [];
+    let refresher = new DirectoryViewRefresher(async (dir) => {
+      listed.push(dir);
+    });
+
+    await refresher.refresh('PersonCard/nested/bob.json');
+
+    assert.deepEqual(listed, ['', 'PersonCard', 'PersonCard/nested']);
+  });
+
+  test('concurrent refreshes of one directory never overlap listings and coalesce into fewer listings than refreshes', async function (assert) {
+    let calls: string[] = [];
+    let inFlight = new Map<string, number>();
+    let maxInFlight = new Map<string, number>();
+    let refresher = new DirectoryViewRefresher(async (dir) => {
+      calls.push(dir);
+      let now = (inFlight.get(dir) ?? 0) + 1;
+      inFlight.set(dir, now);
+      maxInFlight.set(dir, Math.max(maxInFlight.get(dir) ?? 0, now));
+      // A listing takes a round trip; every refresh below arrives during it.
+      await new Promise((r) => setTimeout(r, 5));
+      inFlight.set(dir, now - 1);
+    });
+
+    // Five notifications for files in the same directory arrive at once (a
+    // batch write emits one per file).
+    let names = ['a.json', 'b.json', 'c.json', 'd.json', 'e.json'];
+    await Promise.all(
+      names.map((name) => refresher.refresh(`PersonCard/${name}`)),
+    );
+
+    let count = (dir: string) => calls.filter((c) => c === dir).length;
+    assert.strictEqual(maxInFlight.get(''), 1, 'root listings never overlap');
+    assert.strictEqual(
+      maxInFlight.get('PersonCard'),
+      1,
+      'PersonCard listings never overlap',
+    );
+    // The five refresh() calls reach their first `await` synchronously, so
+    // the first starts the root listing and the other four share one pending
+    // follow-up: the root count is structurally exactly 2. The PersonCard
+    // count depends on how its first listing interleaves with the root
+    // follow-up, so a bound is the honest assertion there.
+    assert.strictEqual(
+      count(''),
+      2,
+      'root listed once, then one follow-up for the four that arrived during it',
+    );
+    let dirBounded = count('PersonCard') <= 3;
+    assert.true(
+      dirBounded,
+      `PersonCard listings stayed bounded (${count('PersonCard')})`,
+    );
+
+    calls.length = 0;
+    await refresher.refresh('PersonCard/f.json');
+    assert.deepEqual(
+      calls,
+      ['', 'PersonCard'],
+      'after everything settles a new refresh lists again',
+    );
+  });
+
+  test('a request that arrives while the follow-up listing is running queues another one', async function (assert) {
+    let resolvers: Array<() => void> = [];
+    let refresher = new DirectoryViewRefresher(
+      (_dir) =>
+        new Promise<void>((resolve) => {
+          resolvers.push(resolve);
+        }),
+    );
+    let started = () => resolvers.length;
+    let untilStarted = async (n: number) => {
+      while (started() < n) {
+        await new Promise((r) => setTimeout(r, 0));
+      }
+    };
+
+    let first = refresher.refresh('a.json'); // starts listing 1
+    let second = refresher.refresh('b.json'); // queues listing 2
+    assert.strictEqual(started(), 1, 'only listing 1 has started');
+
+    resolvers[0]();
+    await untilStarted(2); // listing 2 is now running
+
+    // This request arrives while listing 2 runs; listing 2 started before the
+    // write behind this request, so it must get listing 3, not share 2.
+    let third = refresher.refresh('c.json');
+    resolvers[1]();
+    await untilStarted(3);
+    resolvers[2]();
+
+    await Promise.all([first, second, third]);
+    assert.strictEqual(started(), 3, 'the late request got its own listing');
+  });
+
+  test('one ancestor failing does not stop the rest of the chain', async function (assert) {
+    let listed: string[] = [];
+    let refresher = new DirectoryViewRefresher(async (dir) => {
+      listed.push(dir);
+      if (dir === '') {
+        throw new Error('root EIO');
+      }
+    });
+
+    await assert.rejects(refresher.refresh('a/b/c.json'), /root EIO/);
+    assert.deepEqual(
+      listed,
+      ['', 'a', 'a/b'],
+      'the parent directories were still listed after the root failed',
+    );
+  });
+
+  test('a listing that never settles is timed out and stops blocking later refreshes', async function (assert) {
+    let calls = 0;
+    let refresher = new DirectoryViewRefresher(
+      (_dir) =>
+        new Promise<void>((resolve) => {
+          calls++;
+          if (calls > 1) {
+            resolve();
+          }
+          // the first listing never settles
+        }),
+      { timeoutMs: 20 },
+    );
+
+    await assert.rejects(refresher.refresh('a.json'), /timed out listing ""/);
+    await refresher.refresh('b.json');
+    assert.strictEqual(calls, 2, 'the second refresh got a fresh listing');
+  });
+
+  test('a failed listing rejects that refresh and does not poison later ones', async function (assert) {
+    let attempt = 0;
+    let refresher = new DirectoryViewRefresher(async () => {
+      attempt++;
+      if (attempt === 1) {
+        throw new Error('EIO');
+      }
+    });
+
+    await assert.rejects(refresher.refresh('x.json'), /EIO/);
+    await refresher.refresh('x.json');
+    assert.strictEqual(attempt, 2, 'the second refresh listed again');
+  });
+});

packages/realm-server/tests/index.ts

@@ -310,6 +310,7 @@ const ALL_TEST_FILES: string[] = [
   './realm-registry-reconciler-test',
   './realm-registry-writes-test',
   './realm-file-changes-listener-test',
+  './directory-view-refresher-test',
   './realm-index-updated-listener-test',
   './jobs-finished-listener-test',
   './realm-routing-test',

packages/realm-server/tests/realm-file-changes-listener-test.ts

@@ -10,13 +10,14 @@ import {
 } from '../lib/realm-file-changes-listener.ts';
 
 // Minimal fake `Realm` — the listener calls `.url` (via lookup),
-// `.invalidateCache(path)` for per-path payloads, and `.clearLocalSourceCaches()`
-// for wildcard payloads (CS-11156). Stub both; tests pick whichever they
-// care about.
+// `.invalidateCache(path)` plus `.refreshDirectoryView(path)` for per-path
+// payloads, and `.clearLocalSourceCaches()` for wildcard payloads.
+// Stub all three; tests pick whichever they care about.
 function makeFakeRealm(
   url: string,
   hooks: {
     onInvalidate?: (path: string) => void;
+    onRefreshDirectory?: (path: string) => void;
     onClearAll?: () => void;
   },
 ): Realm {
@@ -25,6 +26,9 @@ function makeFakeRealm(
     invalidateCache(path: string) {
       hooks.onInvalidate?.(path);
     },
+    async refreshDirectoryView(path: string) {
+      hooks.onRefreshDirectory?.(path);
+    },
     clearLocalSourceCaches() {
       hooks.onClearAll?.();
     },
@@ -103,9 +107,11 @@ module(basename(import.meta.filename), function () {
   module('RealmFileChangesListener (dispatch)', function () {
     test('handleNotification forwards to the mounted realm', function (assert) {
       const invalidations: Array<{ url: string; path: string }> = [];
+      const refreshedDirectories: string[] = [];
       const realmA = makeFakeRealm('http://x.test/a/', {
         onInvalidate: (path) =>
           invalidations.push({ url: 'http://x.test/a/', path }),
+        onRefreshDirectory: (path) => refreshedDirectories.push(path),
       });
       const listener = new RealmFileChangesListener({
         dbAdapter: {} as unknown as PgAdapter,
@@ -118,13 +124,45 @@ module(basename(import.meta.filename), function () {
       assert.deepEqual(invalidations, [
         { url: 'http://x.test/a/', path: 'cards/foo.gts' },
       ]);
+      assert.deepEqual(
+        refreshedDirectories,
+        ['cards/foo.gts'],
+        'the peer re-lists the directory holding the written path',
+      );
+    });
+
+    test('handleNotification keeps going when refreshDirectoryView rejects', async function (assert) {
+      const invalidations: string[] = [];
+      const realmA = {
+        url: 'http://x.test/a/',
+        invalidateCache(path: string) {
+          invalidations.push(path);
+        },
+        async refreshDirectoryView() {
+          throw new Error('EIO');
+        },
+        clearLocalSourceCaches() {},
+      } as unknown as Realm;
+      const listener = new RealmFileChangesListener({
+        dbAdapter: {} as unknown as PgAdapter,
+        lookupMountedRealm: (url) =>
+          url === 'http://x.test/a/' ? realmA : undefined,
+      });
+
+      listener.handleNotification('http://x.test/a/:cards/foo.gts');
+      // let the rejected promise settle; an unhandled rejection would fail the run
+      await new Promise((resolve) => setTimeout(resolve, 0));
+
+      assert.deepEqual(invalidations, ['cards/foo.gts']);
     });
 
     test('handleNotification with wildcard payload calls clearLocalSourceCaches and not invalidateCache (CS-11156)', function (assert) {
       const invalidations: string[] = [];
+      const refreshedDirectories: string[] = [];
       const clearAllCount = { value: 0 };
       const realmA = makeFakeRealm('http://x.test/a/', {
         onInvalidate: (path) => invalidations.push(path),
+        onRefreshDirectory: (path) => refreshedDirectories.push(path),
         onClearAll: () => clearAllCount.value++,
       });
       const listener = new RealmFileChangesListener({
@@ -145,6 +183,11 @@ module(basename(import.meta.filename), function () {
         [],
         'invalidateCache not called for wildcard payload',
       );
+      assert.deepEqual(
+        refreshedDirectories,
+        [],
+        'refreshDirectoryView not called for wildcard payload',
+      );
     });
 
     test('handleNotification drops silently when the url is not mounted', function (assert) {