From 26d43483a74093bc5af8883aba38a070fbe485b9 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 8 Aug 2026 06:28:12 +0000 Subject: [PATCH 1/2] test(metadata): pin the writable-loader half of register()'s announcement ordering (#6548) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Claude-Session: https://claude.ai/code/session_01KDU3qAuJyajAQm3GkUXdfA --- .../src/register-notifies-watchers.test.ts | 266 +++++++++++++++++- 1 file changed, 260 insertions(+), 6 deletions(-) diff --git a/packages/metadata/src/register-notifies-watchers.test.ts b/packages/metadata/src/register-notifies-watchers.test.ts index 35fb33fa24..b594c358d0 100644 --- a/packages/metadata/src/register-notifies-watchers.test.ts +++ b/packages/metadata/src/register-notifies-watchers.test.ts @@ -29,17 +29,30 @@ * 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'; +import type { MetadataLoader } from './loaders/loader-interface'; import { DEFAULT_METADATA_TYPE_REGISTRY } from '@objectstack/spec/kernel'; vi.mock('@objectstack/core', () => ({ @@ -68,6 +81,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 { + if (!this.closed) return; + await new Promise((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>(); + 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 { + 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 { + this.store.get(type)?.delete(name); + } + + async load(type: string, name: string): Promise { + return { data: this.peek(type, name) ?? null }; + } + async loadMany(type: string): Promise { + return Array.from(this.store.get(type)?.values() ?? []) as T[]; + } + async exists(type: string, name: string): Promise { + return this.has(type, name); + } + async stat(): Promise { + return null; + } + async list(type: string): Promise { + return Array.from(this.store.get(type)?.keys() ?? []); + } +} + +/** Give queued microtasks a chance to run. */ +const flush = async (): Promise => { + 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 +454,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) => { + 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) => { + seen.push(event); + }); + + await manager.register('object', 'account', { name: 'account' }, { notify: false }); + + expect(seen).toHaveLength(0); + expect(loader.peek('object', 'account')).toEqual({ name: 'account' }); + }); +}); From effd66f76c08757cd61975b383e41bde83346e94 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 8 Aug 2026 06:54:24 +0000 Subject: [PATCH 2/2] test(metadata): keep the new pin out of the package's frozen type-check debt (#6548) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Claude-Session: https://claude.ai/code/session_01KDU3qAuJyajAQm3GkUXdfA --- .../metadata/src/register-notifies-watchers.test.ts | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/packages/metadata/src/register-notifies-watchers.test.ts b/packages/metadata/src/register-notifies-watchers.test.ts index b594c358d0..9dae2d8116 100644 --- a/packages/metadata/src/register-notifies-watchers.test.ts +++ b/packages/metadata/src/register-notifies-watchers.test.ts @@ -52,7 +52,14 @@ import type { } from '@objectstack/spec/system'; import { MetadataManager } from './metadata-manager'; import { MemoryLoader } from './loaders/memory-loader'; -import type { MetadataLoader } from './loaders/loader-interface'; +// `.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', () => ({ @@ -557,7 +564,7 @@ describe('#6548 — register() announces only once every WRITABLE LOADER holds t const manager = managerOver(fast, slow); const seen: MetadataWatchEvent[] = []; - manager.subscribe('object', (event) => { + manager.subscribe('object', (event: MetadataWatchEvent) => { seen.push(event); }); @@ -586,7 +593,7 @@ describe('#6548 — register() announces only once every WRITABLE LOADER holds t const manager = managerOver(loader); const seen: MetadataWatchEvent[] = []; - manager.subscribe('object', (event) => { + manager.subscribe('object', (event: MetadataWatchEvent) => { seen.push(event); });