fix(metadata): revertPackage finds a code-shipped package's members by the _packageId stamp

.changeset/22113-revert-package-code-shipped-members.md

@@ -0,0 +1,19 @@
+---
+'@objectstack/metadata': patch
+'@objectstack/runtime': patch
+---
+
+fix(runtime,metadata): publishing or reverting a read-only package is refused with `422 WRITABLE_PACKAGE_REQUIRED`, and package membership reads the `_packageId` stamp
+
+Clause-②: no
+
+ADR-0070 D2 makes a code or installed package read-only. `POST /api/v1/packages/:id/publish` and `POST /api/v1/packages/:id/revert` did not check this. These two answers change:
+
+- **Publish of a code or installed package: 200 → 422.** Before, the publish door answered 200. For a package whose items carry an authored `packageId`, for example `com.example.showcase`'s two capabilities, it answered `success: true` and wrote `publishedDefinition`, `state` and `version` onto those read-only items. For every other code package it answered `success: false`, "No metadata items found". It now answers `422 WRITABLE_PACKAGE_REQUIRED` before anything is written, the same refusal that `PATCH /packages/:id/disable` and `DELETE /packages/:id` already give.
+- **Revert of a code or installed package: 404 or 409 → 422.** Before, the revert door answered `404 RESOURCE_NOT_FOUND` "No metadata items found" (for example `com.objectstack.setup` and the platform packages that ship objects) or `409 RESOURCE_CONFLICT` "Package '…' has never been published" (for example `com.example.showcase`). It now answers `422 WRITABLE_PACKAGE_REQUIRED`. The check runs after the protocol's stored-row answer, so a revert of a code package that has a stored row bound to it, such as an organization overlay draft, keeps its answer.
+
+To customise what a code package provides, use an ADR-0005 organization overlay. Overlay drafts publish through `POST /api/v1/packages/:id/publish-drafts` and the per-item publish door, and this change leaves both alone.
+
+**Unchanged:** a writable package's publish and revert, and an id that nothing carries (revert 404; publish 200 with `success: false`).
+
+`MetadataManager.publishPackage` and `revertPackage` now find a package's members by `packageId`, `package` or the private `_packageId` stamp. The artifact loader writes that stamp through `applyProtection`, and the ObjectQL object bridge copies it onto every object it registers. Before, an item that carried only the stamp was not a member. `MetadataManager` has no notion of package kind; the refusal of read-only packages lives at the two doors above.

packages/metadata/src/metadata-manager.ts

@@ -1795,6 +1795,47 @@ export class MetadataManager implements IMetadataService {
     }
   }
 
+  /**
+   * [#22113] The registry items that belong to `packageId`, for the two
+   * package-wide writes ({@link publishPackage}, {@link revertPackage}).
+   *
+   * An item belongs to a package when any of three keys names it:
+   *
+   *  - `packageId` — the publish envelope's own key, and what a caller that
+   *    registers an item for a package writes;
+   *  - `package` — the legacy package stamp;
+   *  - `_packageId` — the private provenance stamp. `applyProtection`
+   *    (`@objectstack/spec/shared`, ADR-0010 §3.7) writes it on every item a
+   *    code-shipped artifact registers here, and the ObjectQL object bridge
+   *    copies it onto every object it registers here. A code-shipped item
+   *    carries ONLY this key, and it is the key the metadata protocol scopes
+   *    this registry's items by when it serves a package's reads.
+   *
+   * Reading the first two alone made a package's publish and revert miss
+   * every item that carries only the stamp ("No metadata items found" while
+   * every read served them).
+   *
+   * ⛔ Not a writability check. This class has no package-kind concept, so it
+   * cannot tell a read-only code package (ADR-0070 D2) from a writable one,
+   * and with the stamp read it would publish a code package's items like any
+   * other. The refusal belongs to the caller that holds the predicate: the
+   * `POST /packages/:id/publish` and `/revert` doors (`@objectstack/runtime`)
+   * refuse a non-writable package with `422 WRITABLE_PACKAGE_REQUIRED` before
+   * calling either method.
+   */
+  private collectPackageMembers(packageId: string): Array<{ type: string; name: string; data: any }> {
+    const members: Array<{ type: string; name: string; data: any }> = [];
+    for (const [type, typeStore] of this.registry) {
+      for (const [name, data] of typeStore) {
+        const meta = data as any;
+        if (meta?.packageId === packageId || meta?.package === packageId || meta?._packageId === packageId) {
+          members.push({ type, name, data: meta });
+        }
+      }
+    }
+    return members;
+  }
+
   /**
    * Publish an entire package:
    * 1. Validate all draft items
@@ -1832,16 +1873,10 @@ export class MetadataManager implements IMetadataService {
     const shouldValidate = options?.validate !== false;
     const publishedBy = options?.publishedBy;
 
-    // Collect all items belonging to this package
-    const packageItems: Array<{ type: string; name: string; data: any }> = [];
-    for (const [type, typeStore] of this.registry) {
-      for (const [name, data] of typeStore) {
-        const meta = data as any;
-        if (meta?.packageId === packageId || meta?.package === packageId) {
-          packageItems.push({ type, name, data: meta });
-        }
-      }
-    }
+    // Collect all items belonging to this package — the same membership
+    // `revertPackage` reads. A read-only code package never gets here through
+    // the publish door, which refuses it first (see `collectPackageMembers`).
+    const packageItems = this.collectPackageMembers(packageId);
 
     if (packageItems.length === 0) {
       return {
@@ -2060,15 +2095,7 @@ export class MetadataManager implements IMetadataService {
    * Restores all metadata definitions from their published snapshots.
    */
   async revertPackage(packageId: string): Promise<void> {
-    const packageItems: Array<{ type: string; name: string; data: any }> = [];
-    for (const [type, typeStore] of this.registry) {
-      for (const [name, data] of typeStore) {
-        const meta = data as any;
-        if (meta?.packageId === packageId || meta?.package === packageId) {
-          packageItems.push({ type, name, data: meta });
-        }
-      }
-    }
+    const packageItems = this.collectPackageMembers(packageId);
 
     // [#7559] ADR-0112 — both refusals below carry a DECLARED `code` + `status`.
     // They are the ordinary answers to an ordinary request (revert a package id

packages/metadata/src/metadata-service.test.ts

@@ -822,6 +822,71 @@ describe('MetadataManager — IMetadataService Contract', () => {
         message: expect.stringContaining('has never been published'),
       });
     });
+
+    // [#22113] An item can carry ONLY the private `_packageId` stamp —
+    // `applyProtection` writes it on every item an artifact registers, and the
+    // ObjectQL object bridge copies it onto every object. Collecting members by
+    // `packageId` / `package` alone answered 404 "No metadata items found" for a
+    // package every read serves. (Whether the package may be reverted at all is
+    // the door's question — it refuses a read-only one with 422 first.)
+    it('finds a package by the _packageId stamp: never published ⇒ 409, not 404', async () => {
+      await manager.register('object', 'code_item', {
+        name: 'code_item', label: 'Code Item', _packageId: 'com.acme.code', _provenance: 'package',
+      });
+
+      await expect(manager.revertPackage('com.acme.code')).rejects.toMatchObject({
+        code: 'RESOURCE_CONFLICT',
+        status: 409,
+        message: "Package 'com.acme.code' has never been published",
+      });
+    });
+
+    it('membership is any of packageId, package or _packageId: each member with a snapshot is restored', async () => {
+      const snapshot = (label: string) => ({ label });
+      await manager.register('object', 'by_package_id', {
+        name: 'by_package_id', packageId: 'com.acme.keys', publishedDefinition: snapshot('A'), state: 'draft',
+      });
+      await manager.register('object', 'by_package', {
+        name: 'by_package', package: 'com.acme.keys', publishedDefinition: snapshot('B'), state: 'draft',
+      });
+      await manager.register('object', 'by_stamp', {
+        name: 'by_stamp', _packageId: 'com.acme.keys', publishedDefinition: snapshot('C'), state: 'draft',
+      });
+      // Control: a different package's item is not a member.
+      await manager.register('object', 'other_pkg', {
+        name: 'other_pkg', _packageId: 'com.acme.other', publishedDefinition: snapshot('D'), state: 'draft',
+      });
+
+      await manager.revertPackage('com.acme.keys');
+
+      for (const [name, label] of [['by_package_id', 'A'], ['by_package', 'B'], ['by_stamp', 'C']] as const) {
+        const item = await manager.get('object', name) as any;
+        expect({ name, state: item.state, metadata: item.metadata }).toEqual({ name, state: 'active', metadata: { label } });
+      }
+      const other = await manager.get('object', 'other_pkg') as any;
+      expect(other.state).toBe('draft');
+      expect(other.metadata).toBeUndefined();
+    });
+  });
+
+  // [#22113] `publishPackage` reads the same membership as `revertPackage`:
+  // an item carrying only the stamp is a member. (Flipped from the pin that
+  // held it OUT while nothing refused a read-only code package; the
+  // `POST /packages/:id/publish` door now refuses one with
+  // `422 WRITABLE_PACKAGE_REQUIRED` before this method runs.)
+  describe('publishPackage — an item carrying only the _packageId stamp', () => {
+    it('is a member: it is snapshotted and published', async () => {
+      await manager.register('object', 'stamped_item', {
+        name: 'stamped_item', label: 'Stamped Item', _packageId: 'com.acme.stamped', _provenance: 'package',
+      });
+
+      const result = await manager.publishPackage('com.acme.stamped', { validate: false });
+
+      expect(result).toMatchObject({ success: true, itemsPublished: 1, version: 1 });
+      const item = await manager.get('object', 'stamped_item') as any;
+      expect(item.state).toBe('active');
+      expect(item.publishedDefinition).toMatchObject({ name: 'stamped_item', label: 'Stamped Item' });
+    });
   });
 
   describe('getPublished', () => {

packages/runtime/src/domains/packages.ts

@@ -378,6 +378,12 @@ function requireReadCapability(deps: DomainHandlerDeps, context: HttpProtocolCon
  * authoring one ("switch to a writable package in the package selector") names
  * a remedy that makes no sense for a delete.
  *
+ * [#22113] `POST /packages/:id/publish` and `POST /packages/:id/revert` ask it
+ * too, with the same predicate and refusal: publishing or reverting a code
+ * package's shipped items is an authoring write into a read-only package
+ * (ADR-0070 D2). Revert asks it only after the protocol's stored-row answer, so
+ * a `sys_metadata` row bound to the package keeps its own answer.
+ *
  * [#14451] The remedy it names is ADR-0005 org overlay, and it used to name
  * `POST /packages/:id/duplicate` instead. That sentence sent a caller holding a
  * read-only package at a route which — for exactly the packages this refusal
@@ -1459,6 +1465,16 @@ export async function handlePackagesRequest(deps: DomainHandlerDeps, path: strin
         if (parts.length === 2 && parts[1] === 'publish' && m === 'POST') {
             const denied = requireManageMetadata(deps, _context); if (denied) return denied;
             const id = decodeURIComponent(parts[0]);
+            // [#22113] ADR-0070 D2: a code or installed package is read-only, so
+            // this door refuses it BEFORE `publishPackage`, which would otherwise
+            // snapshot and re-register the package's items as published (measured:
+            // a platform package's objects, and the showcase's capabilities, at
+            // 200). The metadata service has no package-kind concept to refuse
+            // with; the door's one predicate does. An organization overlay draft
+            // on a code package is not published here: it is a `sys_metadata` row
+            // the protocol publishes through `publish-drafts` and the per-item
+            // publish door, neither of which passes this branch.
+            const readOnly = requireWritablePackage(deps, qlService, id, 'publish'); if (readOnly) return readOnly;
             const metadataService = await deps.getService(_context, CoreServiceName.enum.metadata);
             if (metadataService && typeof (metadataService as any).publishPackage === 'function') {
                 const result = await (metadataService as any).publishPackage(id, body || {});
@@ -1917,6 +1933,14 @@ export async function handlePackagesRequest(deps: DomainHandlerDeps, path: strin
                     return { handled: true, response: deps.errorFromThrown(e, 500) };
                 }
             }
+            // [#22113] ADR-0070 D2: a code or installed package has nothing of its
+            // own to revert, so it is refused here, AFTER the protocol's stored-row
+            // answer above. A stored row bound to it (an organization overlay
+            // draft) is still answered by the protocol, exactly as before; only the
+            // metadata service's revert of the package's shipped items is refused.
+            // "Has never been published" would point the caller at publishing,
+            // which the publish door above refuses for the same package.
+            const readOnly = requireWritablePackage(deps, qlService, id, 'revert'); if (readOnly) return readOnly;
             const metadataService = await deps.getService(_context, CoreServiceName.enum.metadata);
             if (metadataService && typeof (metadataService as any).revertPackage === 'function') {
                 await (metadataService as any).revertPackage(id);