fix(metadata)!: the unscoped plural read counts an action name's holders per (object, name), so the boot pass keeps a package-less row on another object beside the holder's

.changeset/32681-plural-read-object-holders.md

@@ -0,0 +1,22 @@
+---
+'@objectstack/metadata': patch
+---
+
+fix(metadata)!: the unscoped plural read counts an action name's holders per `(object, name)`, so a package-less row bound to an object no package binds the name to is the environment's own entry and the boot priming pass registers it beside the holder's, never in its place (#32681)
+
+Clause-②: yes (narrowing) — narrowed: on a host that wires a `DatabaseLoader` (none in the workspace), where one package binds an `action` name to one object (or holds it object-less) and a package-less row binds it to another object (or object-less), after the boot priming pass a read naming that package (`MetadataManager.get`) answers the package's own item, where it answered the package-less row when that row was saved last (a binding the package never declared). Widened: after the pass the registry holds both items, where it held the one saved last alone; `MetadataManager.loadMany`, `loadManyKeyed` and `DatabaseLoader.loadManyKeyed` hand on both entries, where they handed on one.
+Metadata-protocol: none
+
+<!-- adr-0087: not-required (no-migration-prescription) No key of any schema is added, removed, renamed or re-shaped, and no stored row is rewritten: what moves is which stored rows an unscoped plural read keeps apart, which the boot pass then registers, so there is nothing for `objectstack migrate meta` to rewrite. Rows stay as stored. -->
+
+**BREAKING** for one kind of host: a host that wires a `DatabaseLoader`, where package ALPHA binds an action name to `account` and a package-less row binds the same name to `contact`, now answers ALPHA's own `account` item to a read naming ALPHA after the boot, instead of the `contact` row. Read the `contact` row with no package, or save it as ALPHA's (`_packageId`) if it is ALPHA's.
+
+What moves, for `action` (seat ruling A on #32681, ADR-0150 D4), with ALPHA and BETA two packages and `approve` the action name:
+
+- `pluralEntryKeys` (the key `MetadataManager.loadMany`, `MetadataManager.loadManyKeyed` and `DatabaseLoader`'s unscoped reads keep one entry per) folds an entry that carries no package into the name's one holder only when that holder binds the name to the entry's own object (or both are object-less), FROM whatever object either was bound to:
+  - ALPHA on `account`, a package-less row on `contact`: two entries, FROM one (the row saved last). After the boot pass the registry holds `ALPHA|account>approve` and `contact>approve`, FROM `contact>approve` alone (package-less row saved last) or ALPHA's key alone (saved first); a read naming ALPHA answers ALPHA's `account` item, FROM the `contact` row when that row was saved last.
+  - BETA on `contact` alone, a package-less row on `account`: two entries, FROM one. After the pass a read naming BETA answers BETA's `contact` item, FROM the `account` row when that row was saved last.
+  - ALPHA on `account` and a package-less object-less row, or ALPHA object-less and a package-less row on `account`: two entries, FROM one, the same way.
+- Every read after the pass now answers what the same read without the pass answers for each shape above.
+
+Not changed: a name one package holds on one object beside a package-less row on that object (one entry, the row saved last, as before); a name two packages hold (the package-less row keeps its own entry, as before); the `/actions/OBJECT/NAME` route's declaration read (the protocol's execution face, which answers the environment's item on the other object beside one package holder, so the holder's own route still judges no declaration, as before); types other than `action`; stored rows (no migration).

packages/metadata/src/loaders/database-loader.test.ts

@@ -2190,7 +2190,11 @@ describe('[#32791] a stored package-less row the boot pass files under the host\
     await seed('action', null, 2);
     const m = await bootPass(HOST);
     // [#32812] FROM `main` `25ee5076`: the host's key. TO: the bare slot.
-    expect(registryKeys(m, 'action')).toEqual(['helper']);
+    // [#32681] …beside ALPHA's own `account` key: the object-less row is not
+    // ALPHA's customization (ALPHA binds the name to `account`), so the plural
+    // read keeps both rows and the pass registers both, FROM the bare slot
+    // alone (the fold kept the row saved last).
+    expect(registryKeys(m, 'action')).toEqual([`${ALPHA}\u0000account\u0001helper`, 'helper']);
     expect(await bareAnswers(m)).toEqual(['env', 'env']);
   });
 
@@ -3884,3 +3888,126 @@ describe('[#32680] the plural read takes a by-package name\'s bodies and package
     });
   }
 });
+
+/**
+ * [#32681; seat ruling A on #32681, ADR-0150 D4] The unscoped plural read
+ * counts an action name's holders per `(object, name)` (`pluralEntryKeys`): a
+ * stored package-less row bound to an object no package binds the name to is
+ * the environment's own item and keeps an entry of its own, where it was
+ * folded into the name's one holder by name. So the boot priming pass
+ * registers both rows, and every read after the pass answers what it answers
+ * without it.
+ *
+ * BEFORE (measured on main `fc283d6d97`, same fixtures), with the
+ * package-less row saved last: the plural read handed on that row alone, the
+ * registry ended with the package-less key alone, and after the pass a read
+ * naming the holder answered the package-less row (ALPHA: the `contact` row;
+ * BETA: the `account` row) where the read without the pass answered the
+ * holder's own one. Saved first: the plural read handed on the holder's row
+ * alone, and the package-less row was in no registry slot. The CONTROLs
+ * answered as here.
+ */
+describe('[#32681] the plural read counts an action name\'s holders per (object, name): a package-less row on an object no package binds is the environment\'s own', () => {
+  const ALPHA = 'com.alpha.app';
+  const BETA = 'com.beta.app';
+  let driver: IDataDriver;
+
+  /** A binding of `approve`: its package (`null`: none) and its object (`null`: object-less). */
+  type Binding = readonly [packageId: string | null, objectName: string | null];
+  const labelOf = ([packageId, objectName]: Binding) => `${packageId ?? 'env'}@${objectName ?? 'global'}`;
+  /** A stored row of `approve`, bound by its columns, at `version`. */
+  const seed = (binding: Binding, version: number) =>
+    driver.create('sys_metadata', {
+      id: `approve_${labelOf(binding)}`, type: 'action', name: 'approve', state: 'active', version, checksum: `approve_${version}`,
+      source: 'database', package_id: binding[0], bundle_member: binding[1],
+      metadata: JSON.stringify({ name: 'approve', label: labelOf(binding), ...(binding[1] ? { objectName: binding[1] } : {}) }),
+    });
+  const cold = () => {
+    const m = new MetadataManager({ formats: ['json'], loaders: [] });
+    m.registerLoader(new DatabaseLoader({ driver }));
+    return m;
+  };
+  const bootPass = async () => {
+    const plugin = new MetadataPlugin({ watch: false, config: { bootstrap: 'eager' } });
+    const m = (plugin as any).manager as MetadataManager;
+    (m as any).loaders.clear();
+    m.registerLoader(new DatabaseLoader({ driver }));
+    await (plugin as any)._loadFromFileSystem({ logger: { info: vi.fn(), warn: vi.fn(), error: vi.fn(), debug: vi.fn() } });
+    return m;
+  };
+  /** What a read answers: the body's label, or the refusal's code, status, reason and holders. */
+  const answer = async (m: MetadataManager, packageId?: string) => {
+    try {
+      return ((await m.get('action', 'approve', packageId)) as { label?: string } | undefined)?.label;
+    } catch (e: any) {
+      return [e?.code, e?.status, e?.refusal?.reason, ...(e?.refusal?.candidates ?? []).map((c: any) => c.packageId ?? c.kind).sort()];
+    }
+  };
+  /** Every read: bare, naming ALPHA, naming BETA; and the listed labels. */
+  const reads = async (m: MetadataManager) => [
+    ...(await Promise.all([undefined, ALPHA, BETA].map((packageId) => answer(m, packageId)))),
+    ((await m.list('action')) as Array<{ label?: string }>).map((item) => item.label).sort(),
+  ];
+  /** The registry's keys of `approve`, spelled `PACKAGE|OBJECT>name`, sorted. */
+  const registryKeys = (m: MetadataManager) => [...(((m as any).registry.get('action') as Map<string, unknown> | undefined)?.keys() ?? [])]
+    .map((key) => key.replace(/\u0000/g, '|').replace(/\u0001/g, '>')).sort();
+  /** What the unscoped plural read hands on: `[package stamp, label]`, sorted. */
+  const plural = async () => ((await cold().loadMany('action')) as Array<{ _packageId?: string; label?: string }>)
+    .map((item) => [item._packageId ?? null, item.label]).sort((a, b) => JSON.stringify(a).localeCompare(JSON.stringify(b)));
+
+  beforeEach(() => {
+    driver = createMockDriver();
+  });
+
+  const MOVED: ReadonlyArray<[shape: string, holder: Binding, packageLess: Binding, readsAfter: unknown[], keys: string[]]> = [
+    ['mirror shape: ALPHA on account, a package-less row on contact', [ALPHA, 'account'], [null, 'contact'],
+      ['env@contact', `${ALPHA}@account`, 'env@contact', [`${ALPHA}@account`, 'env@contact']], [`${ALPHA}|account>approve`, 'contact>approve']],
+    ['single holder on another object: BETA on contact, a package-less row on account', [BETA, 'contact'], [null, 'account'],
+      ['env@account', 'env@account', `${BETA}@contact`, [`${BETA}@contact`, 'env@account']], [`${BETA}|contact>approve`, 'account>approve']],
+    ['ALPHA on account, an object-less package-less row', [ALPHA, 'account'], [null, null],
+      ['env@global', `${ALPHA}@account`, 'env@global', [`${ALPHA}@account`, 'env@global']], [`${ALPHA}|account>approve`, 'approve']],
+    ['ALPHA object-less, a package-less row on account', [ALPHA, null], [null, 'account'],
+      ['env@account', `${ALPHA}@global`, 'env@account', [`${ALPHA}@global`, 'env@account']], [`${ALPHA}|approve`, 'account>approve']],
+  ];
+  for (const [shape, holder, packageLess, readsAfter, keys] of MOVED) {
+    for (const [order, versions] of [['the package-less row saved last', [1, 2]], ['the package-less row saved first', [2, 1]]] as const) {
+      it(`${shape} (${order}): the plural read hands on both, the pass registers both, and every read after it answers what it answers without it`, async () => {
+        await seed(holder, versions[0]);
+        await seed(packageLess, versions[1]);
+        expect(await plural()).toEqual([[holder[0], labelOf(holder)], [null, labelOf(packageLess)]]);
+        const without = await reads(cold());
+        expect(without).toEqual(readsAfter);
+        const m = await bootPass();
+        expect(registryKeys(m)).toEqual([...keys].sort());
+        expect(await reads(m)).toEqual(without);
+        expect(driver.update).not.toHaveBeenCalled();
+      });
+    }
+  }
+
+  for (const [order, versions] of [['the package-less row saved last', [1, 2]], ['the package-less row saved first', [2, 1]]] as const) {
+    it(`CONTROL a name one package holds on one object: ALPHA on account and a package-less row on account (${order}) are one entry of the plural read, the one saved last, as before`, async () => {
+      await seed([ALPHA, 'account'], versions[0]);
+      await seed([null, 'account'], versions[1]);
+      const last = versions[1] > versions[0] ? [null, 'env@account'] : [ALPHA, `${ALPHA}@account`];
+      expect(await plural()).toEqual([last]);
+      const m = await bootPass();
+      expect(registryKeys(m)).toEqual([last[0] === null ? 'account>approve' : `${ALPHA}|account>approve`]);
+    });
+  }
+
+  it('CONTROL two packages on one object: ALPHA and BETA on account and a package-less row on account are three entries, a bare read is refused naming both and the environment, before and after the pass', async () => {
+    await seed([ALPHA, 'account'], 1);
+    await seed([BETA, 'account'], 2);
+    await seed([null, 'account'], 3);
+    expect(await plural()).toEqual([[ALPHA, `${ALPHA}@account`], [BETA, `${BETA}@account`], [null, 'env@account']]);
+    const without = await reads(cold());
+    expect(without).toEqual([
+      ['RESOURCE_CONFLICT', 409, 'ambiguous-in-environment', ALPHA, BETA, 'environment'], `${ALPHA}@account`, `${BETA}@account`,
+      [`${ALPHA}@account`, `${BETA}@account`, 'env@account'],
+    ]);
+    const m = await bootPass();
+    expect(registryKeys(m)).toEqual(['account>approve', `${ALPHA}|account>approve`, `${BETA}|account>approve`]);
+    expect(await reads(m)).toEqual(without);
+  });
+});

packages/metadata/src/package-slotted-store.ts

@@ -525,10 +525,25 @@ export function bundleEntryKey(type: string, name: string, data: unknown, packag
  *    `MemoryLoader` and on `DatabaseLoader` (whose rows are one per binding
  *    since #32651, its unscoped reads one per name); `list` and the loaders'
  *    `loadManySlots` serve every binding.
+ *
+ * [#32681, ADR-0150 D4] For a type whose identity carries its owner
+ * (`action`), an entry that carries no package is the name's one holder's
+ * only when that holder binds the name to the entry's own object (or holds
+ * the object-less name, for an object-less entry) — the holders of its
+ * `(object, name)` ({@link entryIdentitiesOf}), as the store's write rule
+ * counts them ({@link storeWriteKey}). Bound to an object no package binds
+ * the name to, it is the environment's own item and keeps a key of its own.
+ * FROM the name's one holder whatever object it was bound to: beside ALPHA's
+ * `approve` on `account`, a package-less `approve` on `contact` was folded
+ * into ALPHA's entry, so the unscoped read kept one of the two rows (the one
+ * saved last), and the boot priming pass registered the `contact` row alone
+ * (in the bare slot, a read naming ALPHA then answered it) or ALPHA's alone.
+ * Under a name two or more packages hold the entry keeps a key of its own as
+ * before ({@link pluralEntryCustomizedHolders} names the holder it customizes).
  */
 export function pluralEntryKeys(
   type: string,
-  entries: ReadonlyArray<{ readonly name: unknown; readonly data: unknown; readonly packageId?: string }>,
+  entries: ReadonlyArray<{ readonly name: unknown; readonly data: unknown; readonly packageId?: string; readonly member?: string | null }>,
 ): Array<string | undefined> {
   if (!PACKAGE_SLOTTED_STORE_TYPES.has(type) || isBundledStoreType(type)) {
     return entries.map(({ name, data, packageId }) => (typeof name !== 'string'
@@ -544,14 +559,43 @@ export function pluralEntryKeys(
     if (!held) holders.set(name, (held = new Set()));
     held.add(holder);
   });
+  const identities = OWNER_KEYED_SLOT_TYPES.has(type) ? entryIdentitiesOf(type, entries, packageOf) : undefined;
   return entries.map(({ name }, at) => {
     if (typeof name !== 'string') return undefined;
     const held = holders.get(name);
-    const holder = packageOf[at] ?? (held?.size === 1 ? [...held][0] : undefined);
+    const sole = held?.size === 1 ? [...held][0] : undefined;
+    const customizes = sole !== undefined && (identities === undefined || identities.holders.get(identities.of[at]!)?.has(sole) === true);
+    const holder = packageOf[at] ?? (customizes ? sole : undefined);
     return JSON.stringify([holder ?? null, name]);
   });
 }
 
+/**
+ * [#32681] Each plural-read entry's identity for a type whose identity
+ * carries its owner — the row's member (`storedRowMemberOf`, its column
+ * first), else its body's object — and the packages holding each identity
+ * among the entries (`packageOf`: each entry's package, `undefined` for none).
+ */
+function entryIdentitiesOf(
+  type: string,
+  entries: ReadonlyArray<{ readonly name: unknown; readonly data: unknown; readonly member?: string | null }>,
+  packageOf: ReadonlyArray<string | undefined>,
+): { of: Array<string | undefined>; holders: Map<string, Set<string>> } {
+  const of = entries.map(({ name, data, member }) => (typeof name !== 'string'
+    ? undefined
+    : ownedIdentityOf(type, name, member !== undefined ? member : storedRowMemberOf(type, data))));
+  const holders = new Map<string, Set<string>>();
+  entries.forEach((_, at) => {
+    const holder = packageOf[at];
+    const identity = of[at];
+    if (identity === undefined || holder === undefined) return;
+    let held = holders.get(identity);
+    if (!held) holders.set(identity, (held = new Set()));
+    held.add(holder);
+  });
+  return { of, holders };
+}
+
 /**
  * [#32681, ADR-0150 D4] Of an unscoped plural read's `entries` (and the keys
  * {@link pluralEntryKeys} gave them), the package each entry that carries none
@@ -563,7 +607,8 @@ export function pluralEntryKeys(
  * store's write rule ({@link storeWriteKey}, by `(object, name)`) files it
  * into that holder's slot: a read naming the holder answers it. `undefined`
  * for every other entry — one with a package, one the plural read already
- * folded into its holder's entry, one no package or two packages hold the
+ * folded into its holder's entry (a name one package holds, bound to that
+ * package's object), one no package or two packages hold the
  * `(object, name)` of, and every entry of a type whose identity is its name
  * (a name two packages hold has two holders of that identity too).
  */
@@ -574,22 +619,10 @@ export function pluralEntryCustomizedHolders(
 ): Array<string | undefined> {
   if (!OWNER_KEYED_SLOT_TYPES.has(type)) return entries.map(() => undefined);
   const packageOf = entries.map(({ data, packageId }) => (isPackageCaller(packageId) ? packageId : storedItemPackageId(data)));
-  // An entry's identity: its row's member (`storedRowMemberOf`, column first), else its body's object.
-  const identityOf = entries.map(({ name, data, member }) => (typeof name !== 'string'
-    ? undefined
-    : ownedIdentityOf(type, name, member !== undefined ? member : storedRowMemberOf(type, data))));
-  const holders = new Map<string, Set<string>>();
-  entries.forEach((_, at) => {
-    const holder = packageOf[at];
-    const identity = identityOf[at];
-    if (identity === undefined || holder === undefined) return;
-    let held = holders.get(identity);
-    if (!held) holders.set(identity, (held = new Set()));
-    held.add(holder);
-  });
+  const identities = entryIdentitiesOf(type, entries, packageOf);
   return entries.map(({ name }, at) => {
     if (typeof name !== 'string' || packageOf[at] !== undefined || keys[at] !== JSON.stringify([null, name])) return undefined;
-    const held = holders.get(identityOf[at]!);
+    const held = identities.holders.get(identities.of[at]!);
     return held?.size === 1 ? [...held][0] : undefined;
   });
 }