fix(objectql)!: a field's picklist resolves from the package that declares it; the api index keeps one item per package and name

.changeset/31678-picklist-by-referrer-package.md

@@ -0,0 +1,22 @@
+---
+'@objectstack/objectql': patch
+'@objectstack/metadata': patch
+---
+
+fix(objectql)!: a field's picklist resolves from the package that declares the field, and the api endpoint index keeps one item per package and name
+
+Clause-②: no (narrowing)
+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 shape changes. What moves is which list a field's existing `picklist` name resolves to at runtime, through the package-scoped resolver ADR-0150 D3 rules, so there is nothing for a migration to rewrite; the D10 upgrade report lists each reference the narrowing refuses. -->
+
+**BREAKING** for a field whose `picklist` names a list its own package cannot reach (`@objectstack/objectql`). A field's `picklist` now resolves from the package that declares the field: that package's own list, else the one package in its declared dependency closure (`manifest.dependencies`, plus the platform's `sys` packages) that holds the name, else nothing. Two kinds of reference that used to be served now are not:
+
+- a field of a package whose list only an environment-authored item holds (one created in this environment, bound to no package);
+- a field of a package whose list another package holds, when the field's package does not declare a dependency on it. Two packages shipped in one artifact are no exception: a sibling's list needs a declared dependency too.
+
+Before, either field was served that list's options. After, it is served with no options and its values are refused, and the boot names each one in a warning instead of failing. A package installed after the boot that carries such a reference is refused at install, naming every holder. The fix is one line: add the package that holds the list to the field's package `manifest.dependencies`, or declare the list in the field's own package.
+
+Unchanged: a field whose package holds the list, or depends on the package that holds it, is served exactly as before; a field written only by an environment-authored body (a tenant overlay, an object created in Studio) resolves the name environment-wide, as before; and a `picklistExtensions` entry still adds its options to the one list its `extend` names in the environment, whether or not its package depends on that list's package.
+
+`@objectstack/metadata`: the api endpoint index reads its stored items one per package and name, so two packages' same-named api items are no longer collapsed into one before the matcher reads them. A package-less row under a name one package holds is that package's customization and keeps sharing its slot. While each api name has one package holder, every route answers as before.

packages/cli/src/utils/picklist-references.boot-parity.test.ts

@@ -138,11 +138,17 @@ describe('[#29898] picklist-reference-unknown — the author-time door and the m
   const boundCase = (picklist: string) =>
     caseObj({ subject: { type: 'text', label: 'Subject' }, status: { type: 'select', label: 'Status', picklist } });
 
-  /** One artifact: the core body carries the object, the app body declares `parity_status`. */
+  /**
+   * One artifact: the core body carries the object, the app body declares
+   * `parity_status`. [ADR-0150 D3, child E6; N1 ruled on #31678: the declared
+   * dependency closure only] Forced edit: the core body declares its
+   * dependency on the app body — a sibling it does not depend on is outside
+   * its closure, and the boot refuses that reference.
+   */
   const artifactWith = (picklist: string): AnyRec => ({
     manifest: manifest(STACK_ID),
     packages: [
-      { manifest: { ...manifest('com.example.parity.core'), objects: [boundCase(picklist)] } },
+      { manifest: { ...manifest('com.example.parity.core'), dependencies: { 'com.example.parity.app': '^1.0.0' }, objects: [boundCase(picklist)] } },
       { manifest: { ...manifest('com.example.parity.app'), picklists: [status] } },
     ],
   });

packages/metadata/src/metadata-manager-match-endpoint.test.ts

@@ -263,3 +263,53 @@ describe('#5089 — MetadataManager.matchEndpoint', () => {
     });
   });
 });
+
+/**
+ * [ADR-0150 D4, child E6] The index's item list keeps one slot per
+ * `(package, name)`, not per bare name: a bare-name map let one package's
+ * stored `api` item overwrite another package's same-named item before the
+ * matcher ever saw it. A package-less row under a name one package holds is
+ * that holder's customization and shares its slot (D4).
+ */
+describe('the endpoint index keeps one slot per (package, name) (ADR-0150 D4)', () => {
+  /** A store whose keyed read answers the given rows, in order, the way `sys_metadata` does. */
+  function storeOf(rows: Array<{ name: string; data: Record<string, unknown> }>): MetadataLoader {
+    return {
+      contract: { name: 'rows', protocol: 'datasource:', capabilities: { read: true, write: false } },
+      async load() { return { data: null }; },
+      async loadMany(type: string) { return type === 'api' ? rows.map((r) => r.data) : []; },
+      async loadManyKeyed(type: string) { return type === 'api' ? rows : []; },
+      async exists() { return false; },
+      async stat() { return null; },
+      async list() { return []; },
+    } as unknown as MetadataLoader;
+  }
+  const A_PATH = { method: 'GET', path: '/api/v1/apps/alpha/tasks' };
+  const B_PATH = { method: 'GET', path: '/api/v1/apps/beta/tasks' };
+
+  it('serves two packages\' same-named api items, each on its own route', async () => {
+    const manager = new MetadataManager({ formats: ['json'], loaders: [] });
+    manager.registerLoader(storeOf([
+      { name: 'list_tasks', data: { ...endpoint({ path: A_PATH.path, target: 'a' }), packageId: 'com.acme.alpha' } },
+      { name: 'list_tasks', data: { ...endpoint({ path: B_PATH.path, target: 'b' }), packageId: 'com.acme.beta' } },
+    ]));
+    expect((await manager.matchEndpoint(A_PATH))?.endpoint.target).toBe('a');
+    expect((await manager.matchEndpoint(B_PATH))?.endpoint.target).toBe('b');
+  });
+
+  it('CONTROL — a package-less row under a name one package holds shares that holder\'s slot: the first one read keeps it', async () => {
+    const manager = new MetadataManager({ formats: ['json'], loaders: [] });
+    manager.registerLoader(storeOf([
+      { name: 'list_tasks', data: { ...endpoint({ path: A_PATH.path, target: 'customized' }) } },
+      { name: 'list_tasks', data: { ...endpoint({ path: B_PATH.path, target: 'shipped' }), packageId: 'com.acme.alpha' } },
+    ]));
+    expect((await manager.matchEndpoint(A_PATH))?.endpoint.target).toBe('customized');
+    expect(await manager.matchEndpoint(B_PATH)).toBeUndefined();
+  });
+
+  it('CONTROL — an environment-authored row whose name no package holds is served', async () => {
+    const manager = new MetadataManager({ formats: ['json'], loaders: [] });
+    manager.registerLoader(storeOf([{ name: 'list_tasks', data: endpoint({ path: A_PATH.path, target: 'env' }) }]));
+    expect((await manager.matchEndpoint(A_PATH))?.endpoint.target).toBe('env');
+  });
+});

packages/metadata/src/metadata-manager.ts

@@ -365,6 +365,23 @@ function redactServedDocument(
   return peeled ? { ...result.item, ...bookkeeping } : result.item;
 }
 
+/**
+ * [ADR-0150 D4, child E6] The package a stored or registered item carries:
+ * its stored envelope's `packageId` (flat or under the publish envelope), else
+ * the registry's `_packageId` stamp on the body; `undefined` for an item bound
+ * to no package (the `sys_metadata` provenance sentinel included).
+ */
+function indexedItemPackageId(item: unknown): string | undefined {
+  const peeled = peelStoredEnvelope(item);
+  const fromEnvelope = (peeled.envelope as { packageId?: unknown }).packageId;
+  const body = peeled.body as { _packageId?: unknown } | null | undefined;
+  const stamped = body && typeof body === 'object' ? body._packageId : undefined;
+  for (const candidate of [fromEnvelope, stamped]) {
+    if (typeof candidate === 'string' && candidate !== '' && candidate !== 'sys_metadata') return candidate;
+  }
+  return undefined;
+}
+
 /**
  * Main metadata manager class.
  * Implements IMetadataService contract for unified metadata management.
@@ -612,7 +629,7 @@ export class MetadataManager implements IMetadataService {
     //      `notifyWatchers` without a cache mutation of their own.
     //      Double invalidation is idempotent, so the overlap is free.
     this.endpointMatcher = new EndpointMatcher({
-      listApiItems: () => this.listForIndex(MetadataManager.ENDPOINT_METADATA_TYPE),
+      listApiItems: () => this.listForIndex(MetadataManager.ENDPOINT_METADATA_TYPE, { byPackage: true }),
       logger: this.logger,
     });
     this.subscribe(MetadataManager.ENDPOINT_METADATA_TYPE, () => this.endpointMatcher.invalidate());
@@ -1186,23 +1203,33 @@ export class MetadataManager implements IMetadataService {
     type: string,
     items: Map<string, unknown>
   ): Promise<void> {
+    for (const entry of await this.loaderEntries(loader, type)) {
+      if (items.has(entry.name)) continue;
+      items.set(entry.name, entry.data);
+    }
+  }
+
+  /**
+   * Every named item one loader holds for `type`, in the loader's order, each
+   * with its key: {@link MetadataLoader.loadManyKeyed} where the loader offers
+   * it, else `loadMany` keyed by each body's own `name` (the pre-#14205
+   * reading, for loaders that cannot produce keys). An item with no key is
+   * left out, as {@link admitLoaderItems} always left it.
+   */
+  private async loaderEntries(loader: MetadataLoader, type: string): Promise<Array<{ name: string; data: unknown }>> {
+    const entries: Array<{ name: string; data: unknown }> = [];
     if (typeof loader.loadManyKeyed === 'function') {
-      const keyed = await loader.loadManyKeyed(type);
-      for (const entry of keyed) {
+      for (const entry of await loader.loadManyKeyed(type)) {
         if (!entry || typeof entry.name !== 'string' || entry.name === '') continue;
-        if (items.has(entry.name)) continue;
-        items.set(entry.name, entry.data);
+        entries.push({ name: entry.name, data: entry.data });
       }
-      return;
+      return entries;
     }
-
-    const loaderItems = await loader.loadMany(type);
-    for (const item of loaderItems) {
+    for (const item of await loader.loadMany(type)) {
       const itemAny = item as { name?: unknown } | null;
-      if (itemAny && typeof itemAny.name === 'string' && !items.has(itemAny.name)) {
-        items.set(itemAny.name, item);
-      }
+      if (itemAny && typeof itemAny.name === 'string' && itemAny.name !== '') entries.push({ name: itemAny.name, data: item });
     }
+    return entries;
   }
 
   private async readListUncached(type: string): Promise<ListReadResult> {
@@ -1403,7 +1430,8 @@ export class MetadataManager implements IMetadataService {
    * driver behind a real `DatabaseLoader` makes `matchEndpoint` reject rather
    * than answer a 404-shaped `undefined`.)
    */
-  private async listForIndex(type: string): Promise<unknown[]> {
+  private async listForIndex(type: string, options?: { byPackage?: boolean }): Promise<unknown[]> {
+    if (options?.byPackage) return this.listForPackageScopedIndex(type);
     const items = new Map<string, unknown>();
 
     const typeStore = this.registry.get(type);
@@ -1421,6 +1449,60 @@ export class MetadataManager implements IMetadataService {
     return Array.from(items.values());
   }
 
+  /**
+   * [ADR-0150 D4, child E6] {@link listForIndex} for an index whose items are
+   * addressed by `(package, name)` — the `api` endpoint index. Same sources,
+   * same precedence (the in-memory registry first, then each loader in turn,
+   * the first entry of a slot kept) and the same refusal to swallow a failed
+   * read; what changes is the SLOT. A bare-name `Map` lets one package's item
+   * overwrite another package's same-named item before the matcher ever sees
+   * it, so each slot is `(package, name)` instead:
+   *
+   *  - an item that carries its package (`packageId` on its stored envelope, or
+   *    the registry's `_packageId` stamp) fills that package's slot;
+   *  - an item that carries none, under a name exactly one package holds here,
+   *    is that holder's customization (ADR-0150 D4: a package-less row bearing
+   *    a package-held name is never a second item), so it shares the holder's
+   *    slot and the precedence above decides between them, as the bare-name
+   *    map did;
+   *  - an item that carries none, under a name no package holds, is the
+   *    environment's own (D1 bucket 4) and fills the environment's slot.
+   *
+   * While phase 1 keeps one package holder per `api` name, every slot holds
+   * what the bare-name map held, so the index answers what it answered. No
+   * name is resolved here: a request carries no caller package, and the
+   * matcher keys its routes by METHOD + path (bucket 1).
+   */
+  private async listForPackageScopedIndex(type: string): Promise<unknown[]> {
+    const entries: Array<{ name: string; data: unknown }> = [];
+    const typeStore = this.registry.get(type);
+    if (typeStore) {
+      for (const [name, data] of typeStore) entries.push({ name, data });
+    }
+    for (const loader of this.loaders.values()) {
+      // No try/catch, on purpose — see {@link listForIndex}.
+      entries.push(...(await this.loaderEntries(loader, type)));
+    }
+
+    const holders = new Map<string, Set<string>>();
+    for (const { name, data } of entries) {
+      const packageId = indexedItemPackageId(data);
+      if (packageId === undefined) continue;
+      let held = holders.get(name);
+      if (!held) holders.set(name, (held = new Set()));
+      held.add(packageId);
+    }
+
+    const slots = new Map<string, unknown>();
+    for (const { name, data } of entries) {
+      const held = holders.get(name);
+      const slotPackage = indexedItemPackageId(data) ?? (held?.size === 1 ? [...held][0] : '');
+      const slot = `${slotPackage}\u0000${name}`;
+      if (!slots.has(slot)) slots.set(slot, data);
+    }
+    return Array.from(slots.values());
+  }
+
   /**
    * Unregister/remove a metadata item by type and name.
    * Deletes from database-backed loaders only (same rationale as register()).
@@ -2197,7 +2279,9 @@ export class MetadataManager implements IMetadataService {
     if (packageEndpointNames.size === 0 && packageFlowNames.size === 0) return [];
 
     const bodies = (items: unknown[]) => items.map((item) => peelStoredEnvelope(item).body);
-    const endpoints = bodies(await this.listForIndex(MetadataManager.ENDPOINT_METADATA_TYPE)) as AnonymousFlowDoorCandidate[];
+    const endpoints = bodies(
+      await this.listForIndex(MetadataManager.ENDPOINT_METADATA_TYPE, { byPackage: true }),
+    ) as AnonymousFlowDoorCandidate[];
     const flows = bodies(await this.listForIndex(MetadataManager.FLOW_METADATA_TYPE)) as ElevatedFlowCandidate[];
 
     const errors: Array<{ type: string; name: string; message: string }> = [];