test(metadata): pin the writable-loader half of register()'s announcement ordering (#6548) (#6607)

* test(metadata): pin the writable-loader half of register()'s announcement ordering (#6548)

register() documents that it announces "once the write has landed in the
registry and every writable loader". After #6043/PR #6547 only the registry
half was observable: this file's sole loader is a MemoryLoader (`memory:`)
and register() persists to `datasource:` loaders only, so hoisting the whole
notifyWatchers block above the save loop left every case green.

Adds a writable `datasource:` fixture backed by a real store (donor shape from
metadata-manager-unregister-invalidate-order.test.ts) and peeks that store
SYNCHRONOUSLY inside the watcher callback — the same instant #6043 used for
the registry. Five cases: single loader, overwrite (readable failure), every
loader rather than the first, an in-flight save holding the announcement back,
and { notify: false } silence with the loader write still landing.

No source change: the loader half is pinned, not narrowed.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KDU3qAuJyajAQm3GkUXdfA

* test(metadata): keep the new pin out of the package's frozen type-check debt (#6548)

Measured: as first written the five cases added 3 raw tsc errors to
@objectstack/metadata's DEBT ledger entry (1 TS2835 from an extensionless
`./loaders/loader-interface` import, 2 TS7006 from the un-annotated watcher
callbacks the unresolved `./metadata-manager` import cascades into).

Spelling the new import with `.js` and annotating the two callbacks with
MetadataWatchEvent puts the file back on its origin/main composition exactly —
2 TS2835 + 14 TS7006, unchanged — so the pin adds zero debt. The `.js` also
makes `implements MetadataLoader` on the fixture a real check rather than a
comment over an `any`. The two pre-existing extensionless imports are #4311's
ledger and are left alone.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KDU3qAuJyajAQm3GkUXdfA

---------

Co-authored-by: Claude <noreply@anthropic.com>

Author: baozhoutao

packages/metadata/src/register-notifies-watchers.test.ts

@@ -29,17 +29,37 @@
  * shape could not pin it. It is now asserted where the ordering is decided:
  * SYNCHRONOUSLY, inside the callback, against the registry `register()` writes.
  *
- * Scope note: `register()` claims the announcement follows the write into the
- * registry AND into every writable loader. Only the registry half is pinned
- * here — this fixture's `MemoryLoader` declares `memory:`, and `register()`
- * persists to `datasource:` loaders only, so no loader in this file is ever
- * written to and the second half is not observable from it. Tracked separately;
- * it needs a writable-datasource fixture rather than another assertion.
+ * [#6548] `register()` claims the announcement follows the write into the
+ * registry AND into every writable loader. After #6043 only the REGISTRY half
+ * was pinned: the suite's shared `MemoryLoader` declares `memory:` and
+ * `register()` persists to `datasource:` loaders only, so no loader in the file
+ * was ever written to — hoisting the whole `notifyWatchers(...)` block above the
+ * save loop, a violation of the documented ordering, left all 15 cases GREEN.
+ * The loader half is now pinned too, in the last describe block below, with the
+ * same shape one store over: a writable `datasource:` fixture whose `save()`
+ * lands the row before it resolves, peeked SYNCHRONOUSLY inside the watcher
+ * callback. Nothing about the guarantee was narrowed — both halves are asserted,
+ * so the comment on `register()` is now enforced as written.
  */
 
 import { describe, it, expect, vi, beforeEach } from 'vitest';
+import type {
+  MetadataLoadResult,
+  MetadataLoaderContract,
+  MetadataSaveResult,
+  MetadataStats,
+  MetadataWatchEvent,
+} from '@objectstack/spec/system';
 import { MetadataManager } from './metadata-manager';
 import { MemoryLoader } from './loaders/memory-loader';
+// `.js` deliberately, unlike the three extensionless imports above it: under
+// `moduleResolution: nodenext` an extensionless relative import does not
+// resolve, and every symbol it names silently becomes `any` (AGENTS.md, the
+// TS7006 cascade). Spelling this one correctly is what makes `implements
+// MetadataLoader` on the fixture below an actual check rather than decoration.
+// The three above are this package's pre-existing type-check debt (#4311) and
+// are left for whoever pays that ledger down.
+import type { MetadataLoader } from './loaders/loader-interface.js';
 import { DEFAULT_METADATA_TYPE_REGISTRY } from '@objectstack/spec/kernel';
 
 vi.mock('@objectstack/core', () => ({
@@ -68,6 +88,105 @@ const registryHas = (mgr: MetadataManager, type: string, name: string): boolean
 const registryPeek = (mgr: MetadataManager, type: string, name: string): unknown =>
   registryOf(mgr).get(type)?.get(name);
 
+/**
+ * [#6548] A gate a test opens by hand, so nothing depends on timer ordering.
+ * Donor shape, verbatim: `metadata-manager-unregister-invalidate-order.test.ts`.
+ */
+class Gate {
+  private parked: Array<() => void> = [];
+  closed = false;
+  async pass(): Promise<void> {
+    if (!this.closed) return;
+    await new Promise<void>((resolve) => this.parked.push(resolve));
+  }
+  get parkedCount(): number {
+    return this.parked.length;
+  }
+  releaseAll(): void {
+    const parked = this.parked;
+    this.parked = [];
+    for (const resolve of parked) resolve();
+  }
+}
+
+/**
+ * [#6548] A writable `datasource:` loader backed by a real store — the ONLY
+ * shape `register()` actually persists into, and therefore the only fixture
+ * from which the loader half of the ordering claim is observable at all. Donor:
+ * `StoreLoader` in `metadata-manager-unregister-invalidate-order.test.ts`,
+ * trimmed to the write path (that file's read/delete gates model an
+ * `unregister()` race this file has no case for).
+ *
+ * `save()` awaits before it seeds, deliberately. A real store write is async,
+ * and the hop is what makes the synchronous peek below able to SEE a
+ * `register()` that fired the save without awaiting it: the row would not be in
+ * the store yet at broadcast time. It still lands the write before it resolves,
+ * so with the gate open every assertion is deterministic rather than timed.
+ */
+class StoreLoader implements MetadataLoader {
+  readonly contract: MetadataLoaderContract;
+  /** type → name → data. */
+  readonly store = new Map<string, Map<string, unknown>>();
+  readonly writeGate = new Gate();
+
+  constructor(name = 'db') {
+    this.contract = {
+      name,
+      protocol: 'datasource:',
+      capabilities: { read: true, write: true, watch: false, list: true },
+    };
+  }
+
+  has(type: string, name: string): boolean {
+    return this.store.get(type)?.has(name) ?? false;
+  }
+
+  peek(type: string, name: string): unknown {
+    return this.store.get(type)?.get(name);
+  }
+
+  async save(type: string, name: string, data: unknown): Promise<MetadataSaveResult> {
+    await this.writeGate.pass();
+    if (!this.store.has(type)) this.store.set(type, new Map());
+    this.store.get(type)!.set(name, data);
+    return { success: true };
+  }
+
+  // Required by `assertWritableLoaderContract` for a writable `datasource:`
+  // loader (#5276/#5654) — this file never drives it.
+  async delete(type: string, name: string): Promise<void> {
+    this.store.get(type)?.delete(name);
+  }
+
+  async load(type: string, name: string): Promise<MetadataLoadResult> {
+    return { data: this.peek(type, name) ?? null };
+  }
+  async loadMany<T = unknown>(type: string): Promise<T[]> {
+    return Array.from(this.store.get(type)?.values() ?? []) as T[];
+  }
+  async exists(type: string, name: string): Promise<boolean> {
+    return this.has(type, name);
+  }
+  async stat(): Promise<MetadataStats | null> {
+    return null;
+  }
+  async list(type: string): Promise<string[]> {
+    return Array.from(this.store.get(type)?.keys() ?? []);
+  }
+}
+
+/** Give queued microtasks a chance to run. */
+const flush = async (): Promise<void> => {
+  for (let i = 0; i < 8; i++) await Promise.resolve();
+};
+
+const managerOver = (...loaders: MetadataLoader[]): MetadataManager => {
+  const mgr = new MetadataManager({ formats: ['json'], loaders: [] });
+  for (const loader of loaders) mgr.registerLoader(loader);
+  mgr.setTypeRegistry(DEFAULT_METADATA_TYPE_REGISTRY);
+  return mgr;
+};
+
 describe('#3112 — register()/unregister() notify subscribe() watchers', () => {
   let manager: MetadataManager;
 
@@ -342,3 +461,145 @@ describe('#3112 — register()/unregister() notify subscribe() watchers', () =>
     });
   });
 });
+
+/**
+ * #6548 — the OTHER half of the same sentence.
+ *
+ * `register()` promises the announcement lands "once the write has landed in the
+ * registry and every writable loader". #6043 pinned the registry half where the
+ * ordering is decided — synchronously, inside the callback — and said in its own
+ * scope note that the loader half was not observable from a fixture whose only
+ * loader speaks `memory:`. It was not observable anywhere else either: hoisting
+ * the announcement above the save loop kept the whole file green.
+ *
+ * These cases close that. Same instant, same technique, one store over: the peek
+ * is at the LOADER's store, taken synchronously inside the watcher callback, so
+ * no `await` hop of any consumer method can move the verdict.
+ *
+ * Why it is worth pinning even though no consumer reads through a loader today
+ * (`get()` resolves against the registry, which outranks every loader): the
+ * ordering is what the method DECLARES, and a declaration nothing can falsify is
+ * the shape this repo keeps paying to rediscover. The cost of the gap was
+ * already concrete once — #5840 abandoned a correct refactor because the old,
+ * frame-counting version of the registry case went red on it while the real
+ * invariant went unmeasured.
+ */
+describe('#6548 — register() announces only once every WRITABLE LOADER holds the write', () => {
+  it('the loader store already holds the new body at broadcast time', async () => {
+    const loader = new StoreLoader();
+    const manager = managerOver(loader);
+
+    const atBroadcast: Array<{ had: boolean; body: unknown }> = [];
+    manager.subscribe('object', () => {
+      atBroadcast.push({
+        had: loader.has('object', 'account'),
+        body: loader.peek('object', 'account'),
+      });
+    });
+
+    await manager.register('object', 'account', { name: 'account', label: 'Fresh' });
+
+    expect(atBroadcast).toHaveLength(1);
+    expect(atBroadcast[0].had).toBe(true);
+    expect(atBroadcast[0].body).toEqual({ name: 'account', label: 'Fresh' });
+    // The control: the fixture really is on the persistence path, so a green
+    // `had: true` above cannot be a loader that was never written to.
+    expect(loader.peek('object', 'account')).toEqual({ name: 'account', label: 'Fresh' });
+  });
+
+  it('on an OVERWRITE too — never with the pre-write row still in the store', async () => {
+    // The overwrite half exists so the failure is READABLE, exactly as in the
+    // registry case above: on a first registration an early announcement reads
+    // `undefined`, which is also what "no loader involved" reads as; here the
+    // pre-write row is a distinct value, so announcing too early fails with 'V1'
+    // where 'V2' was required and names the defect on sight.
+    const loader = new StoreLoader();
+    const manager = managerOver(loader);
+    await manager.register('object', 'account', { name: 'account', label: 'V1' }, { notify: false });
+    expect(loader.peek('object', 'account')).toEqual({ name: 'account', label: 'V1' });
+
+    let bodyAtBroadcast: unknown;
+    manager.subscribe('object', () => {
+      bodyAtBroadcast = loader.peek('object', 'account');
+    });
+
+    await manager.register('object', 'account', { name: 'account', label: 'V2' });
+
+    expect(bodyAtBroadcast).toEqual({ name: 'account', label: 'V2' });
+  });
+
+  it('EVERY writable loader, not merely the first — both stores hold it at broadcast time', async () => {
+    // The word in the comment is "every". An announcement moved INSIDE the save
+    // loop would satisfy a single-loader case and fail here, with the second
+    // store still empty.
+    const first = new StoreLoader('db_a');
+    const second = new StoreLoader('db_b');
+    const manager = managerOver(first, second);
+
+    const atBroadcast: Array<{ a: unknown; b: unknown }> = [];
+    manager.subscribe('object', () => {
+      atBroadcast.push({
+        a: first.peek('object', 'account'),
+        b: second.peek('object', 'account'),
+      });
+    });
+
+    await manager.register('object', 'account', { name: 'account', label: 'Fresh' });
+
+    expect(atBroadcast).toHaveLength(1);
+    expect(atBroadcast[0]).toEqual({
+      a: { name: 'account', label: 'Fresh' },
+      b: { name: 'account', label: 'Fresh' },
+    });
+  });
+
+  it('a loader save still IN FLIGHT holds the announcement back — the save window broadcasts nothing', async () => {
+    // The temporal statement the peeks above cannot make on their own: not
+    // "when it fired, the store was written" but "it had not fired yet while a
+    // writable loader was still mid-write". Parking one store's save opens that
+    // window by hand, so the assertion does not depend on how many microtask
+    // hops `register()` happens to have.
+    const fast = new StoreLoader('db_fast');
+    const slow = new StoreLoader('db_slow');
+    const manager = managerOver(fast, slow);
+
+    const seen: MetadataWatchEvent[] = [];
+    manager.subscribe('object', (event: MetadataWatchEvent) => {
+      seen.push(event);
+    });
+
+    slow.writeGate.closed = true;
+    const write = manager.register('object', 'account', { name: 'account', label: 'Fresh' });
+    await flush();
+
+    // Inside the window: one store written, the other parked mid-save.
+    expect(slow.writeGate.parkedCount).toBe(1);
+    expect(fast.has('object', 'account')).toBe(true);
+    expect(slow.has('object', 'account')).toBe(false);
+    expect(seen).toHaveLength(0);
+
+    slow.writeGate.releaseAll();
+    await write;
+
+    expect(seen).toHaveLength(1);
+    expect(seen[0]).toMatchObject({ type: 'added', metadataType: 'object', name: 'account' });
+    expect(slow.peek('object', 'account')).toEqual({ name: 'account', label: 'Fresh' });
+  });
+
+  it('{ notify: false } still suppresses the announcement — and the loader write still lands', async () => {
+    // Silence is opt-in, never "skipped": the half this file pins for the
+    // registry, asserted for the store the loader half is about.
+    const loader = new StoreLoader();
+    const manager = managerOver(loader);
+
+    const seen: MetadataWatchEvent[] = [];
+    manager.subscribe('object', (event: MetadataWatchEvent) => {
+      seen.push(event);
+    });
+
+    await manager.register('object', 'account', { name: 'account' }, { notify: false });
+
+    expect(seen).toHaveLength(0);
+    expect(loader.peek('object', 'account')).toEqual({ name: 'account' });
+  });
+});