fix(objectql,metadata-protocol): a deleted runtime-created overlay leaves the registry, so list/get/dispatch agree (#5079)

.changeset/meta-delete-retires-overlay-registry-entry.md

@@ -0,0 +1,53 @@
+---
+"@objectstack/objectql": patch
+"@objectstack/metadata-protocol": patch
+---
+
+fix(objectql,metadata-protocol): deleting a runtime-created overlay retires its registry entry, so list/get/dispatch agree (#5079)
+
+Deleting a metadata item an admin had **created** at runtime (`DELETE
+/api/v1/meta/<type>/<name>` for a name no code package ships) removed the
+`sys_metadata` row and reported `reset: true`, while every read surface kept
+serving the deleted item for the life of the process: `GET /meta/<type>` still
+enumerated it, `GET /meta/<type>/<name>` still returned its body, and the
+ADR-0110 D3 declaration gate still resolved a declaration for it. No TTL was
+involved — only a restart cleared it. This is the residual branch of #4432
+("every surface in agreement"), the mirror image of the write direction #4521
+fixed.
+
+**Cause.** #4521 made `saveMetaItem` write an overlay through into the engine's
+`SchemaRegistry` under the PLAIN key, so a saved item is dispatchable and not
+merely listable. The delete side's registry heal
+(`restoreArtifactRegistryView`) only knew how to *un-shadow a packaged
+artifact*: `SchemaRegistry.removeRuntimeShadow` deletes the plain key **only**
+when a composite `<packageId>:<name>` artifact remains underneath, so that the
+name stays resolvable. For a runtime-created item there is no artifact —
+the row *was* the item — so the heal declined and nothing else ever removed the
+entry.
+
+**Fix — at the producer, not the readers.** `restoreArtifactRegistryView` now
+walks the layers under the deleted overlay and stops at the first one that can
+serve the name: (1) a composite-key artifact, (2) a MetadataService baseline,
+and (3) — new — nothing, in which case the plain-key entry is retired via the
+new `SchemaRegistry.removeOverlayEntry(type, name)`. The registry now makes the
+same distinction the delete receipt already makes (#5927): "reset to artifact
+default" vs "it no longer exists".
+
+Two boundaries are preserved deliberately:
+
+- **A packaged artifact is never unregistered.** `removeOverlayEntry` refuses a
+  plain-key entry that is itself an artifact (`_packageId` set, not the
+  `sys_metadata` rehydration sentinel, not tenant-authored) — the same
+  predicate `getArtifactItem` applies to its own bare-key fallback — and never
+  touches composite keys. Resetting a customization of a shipped item still
+  reveals the shipped value.
+- **An outage is not an absence (ADR-0110 D3).** The layer-2 baseline read now
+  decides whether an entry is retired, so it goes through the diagnosed read: a
+  metadata plane that could not answer stops the walk instead of retiring an
+  entry on the strength of a read that never happened.
+
+Measured on the showcase app: before, `POST /api/v1/actions/<object>/<name>`
+after the delete answered 404 with the *handler-miss* wording ("… not found"),
+because the declaration was still resolvable from the stale entry; it now
+answers the ADR-0110 "has no declaration" 404 — byte-identical to the state
+before the item was ever created.

packages/metadata-protocol/src/protocol.ts

@@ -7613,7 +7613,8 @@ export class ObjectStackProtocolImplementation implements
 
     /**
      * Heal the in-memory registry after a metadata reset (overlay-row
-     * delete) on control-plane kernels. Two layers:
+     * delete). Walks the layers UNDER the deleted overlay, in order, and
+     * stops at the first one that can serve the name:
      *
      *  1. Drop the plain-key runtime shadow so the packaged artifact
      *     (registered under `<packageId>:<name>`) becomes the visible
@@ -7626,42 +7627,77 @@ export class ObjectStackProtocolImplementation implements
      *     MetadataService baseline (FilesystemLoader-sourced types) and
      *     re-register it, preserving the historical refresh behaviour
      *     for items the SchemaRegistry never held as artifacts.
+     *  3. [#5079] When NEITHER layer has anything, the deleted row was the
+     *     whole item — so the plain-key entry is retired too
+     *     ({@link SchemaRegistry.removeOverlayEntry}).
+     *
+     * ## Why step 3 exists (#5079, the #4432 residual)
+     *
+     * Step 1 declines for a runtime-CREATED item: `removeRuntimeShadow` only
+     * un-shadows a packaged artifact, and there is none. Step 2 then found
+     * nothing either — and the method returned, leaving the plain-key entry
+     * that #4521's write-through had put there. Nothing else ever removed it,
+     * so for the life of the process `GET /meta/<type>` kept enumerating a
+     * deleted item, `GET /meta/<type>/<name>` kept serving its body, and the
+     * ADR-0110 D3 declaration gate kept resolving it — while the row was gone
+     * from `sys_metadata` and the handler registry had already dropped it.
+     * The measured symptom: after `DELETE /meta/action/x`, `POST
+     * /actions/<obj>/x` 404s with the *handler-miss* wording ("not found")
+     * instead of ADR-0110's "has no declaration", because the declaration was
+     * still resolvable from this stale entry. The delete's own receipt already
+     * tells the truth here — #5927 splits it into "reset to artifact default"
+     * (artifact-backed) vs "it no longer exists" (runtime-only); step 3 is the
+     * registry making the same distinction the receipt makes.
+     *
+     * ## Why the layer-2 read is now diagnosed, and runs on every kernel
+     *
+     * [#5840] left this read on plain `get` because it "decides nothing" —
+     * true then, false now: its `undefined` is what licenses step 3 to retire
+     * an entry. So it goes through {@link readItemFromMetadataService}, which
+     * carries the ADR-0110 D3 verdict, and a DEGRADED read stops the walk
+     * without retiring anything. Retiring on an outage would answer "this
+     * item exists in no layer" from a read that never reached one — the exact
+     * miss-vs-outage confusion #5532/#5840 closed on the sibling paths. The
+     * same helper also folds in the singular/plural retry, so a baseline
+     * stored under the twin spelling is found rather than retired.
+     *
+     * RE-REGISTRATION stays control-plane-only (`environmentId === undefined`)
+     * — the historical refresh semantics of the original call sites, unchanged.
+     * Only the READ is now unconditional, because a project kernel needs the
+     * same evidence before retiring an entry.
      *
      * Best-effort: a failure must never block the delete that already
      * succeeded; the next full reload fixes the registry anyway.
      */
     private async restoreArtifactRegistryView(type: string, name: string): Promise<void> {
         try {
             const registry: any = this.engine.registry;
+            const singular = PLURAL_TO_SINGULAR[type] ?? type;
             let healed = false;
             if (typeof registry.removeRuntimeShadow === 'function') {
-                const singular = PLURAL_TO_SINGULAR[type] ?? type;
                 healed = registry.removeRuntimeShadow(singular, name);
                 if (type !== singular) {
                     healed = registry.removeRuntimeShadow(type, name) || healed;
                 }
             }
             if (healed) return;
-            // MetadataService re-registration is control-plane-only — it
-            // preserves the historical refresh semantics gated on
-            // `environmentId === undefined` at the original call sites.
-            if (this.environmentId !== undefined) return;
-            const services = this.getServicesRegistry?.();
-            const metadataService = services?.get('metadata');
-            if (metadataService && typeof metadataService.get === 'function') {
-                // [#5840] Measured and deliberately left on plain `get`. This
-                // read decides nothing and asserts nothing: it returns void,
-                // its `undefined` produces no answer to any caller, and the
-                // method's own contract above is "best-effort, the next full
-                // reload fixes the registry anyway". Routing it through
-                // `getDiagnosed` could only add a log line to a path that is
-                // already documented as silent — over-applying the rule, which
-                // is how `error`/`warn` become unreadable (AGENTS.md
-                // "Degradation log levels", the do-not-over-apply half).
-                const artifactItem = await metadataService.get(type, name);
-                if (artifactItem !== undefined) {
-                    this.engine.registry.registerItem(type, artifactItem, 'name');
+
+            const baseline = await this.readItemFromMetadataService(type, name);
+            if (baseline.data !== undefined && baseline.data !== null) {
+                if (this.environmentId === undefined) {
+                    this.engine.registry.registerItem(type, baseline.data, 'name');
                 }
+                return;
+            }
+            // ADR-0110 D3 — an outage is not an absence. Leave the entry: it
+            // is stale, which is exactly where this method already was, and a
+            // later delete or reload heals it.
+            if (baseline.degraded) return;
+
+            // [#5079] No artifact, no baseline: the row WAS the item.
+            if (typeof registry.removeOverlayEntry === 'function') {
+                registry.removeOverlayEntry(singular, name);
+                if (type !== singular) registry.removeOverlayEntry(type, name);
             }
         } catch {
             // Best-effort registry refresh; next read fixes it anyway

packages/objectql/src/protocol-registry-shadow.test.ts

@@ -322,3 +322,67 @@ describe('SchemaRegistry.getArtifactItem / removeRuntimeShadow', () => {
         expect((registry.getItem('app', 'mine') as any)?.label).toBe('Mine');
     });
 });
+
+/**
+ * [#5079] The other half of the reset heal — the case
+ * {@link SchemaRegistry.removeRuntimeShadow} above deliberately declines.
+ *
+ * A runtime-CREATED item has no packaged artifact under a composite key, so
+ * `removeRuntimeShadow` leaves its plain-key entry standing (pinned directly
+ * above, and correct: that method's job is un-shadowing an artifact). Since
+ * #4521's write-through puts such an item in the registry, nothing else ever
+ * removed it — `getMetaItems` kept enumerating a deleted item for the life of
+ * the process. `removeOverlayEntry` is what `deleteMetaItem` calls once both
+ * lower layers have answered "nothing".
+ */
+describe('SchemaRegistry.removeOverlayEntry', () => {
+    it('removes the plain-key entry of a runtime-only item', () => {
+        const registry = new SchemaRegistry({ multiTenant: false });
+        registry.registerItem('app', { name: 'mine', label: 'Mine' }, 'name');
+
+        expect(registry.removeOverlayEntry('app', 'mine')).toBe(true);
+        expect(registry.getItem('app', 'mine')).toBeUndefined();
+        expect(registry.listItems('app')).toEqual([]);
+    });
+
+    it('removes a `sys_metadata`-sentinel rehydration entry', () => {
+        // `loadMetaFromDb` stamps the sentinel on package-less overlay rows;
+        // it marks the entry as a rehydration, not a shipped artifact.
+        const registry = new SchemaRegistry({ multiTenant: false });
+        registry.registerItem('app', { name: 'hydrated', label: 'Hydrated', _packageId: 'sys_metadata' }, 'name');
+
+        expect(registry.removeOverlayEntry('app', 'hydrated')).toBe(true);
+        expect(registry.getItem('app', 'hydrated')).toBeUndefined();
+    });
+
+    it('removes a tenant-authored entry even when it carries a real package id', () => {
+        const registry = new SchemaRegistry({ multiTenant: false });
+        registry.registerItem('app', { name: 'org_authored', label: 'Org', _packageId: PKG, _provenance: 'org' }, 'name');
+
+        expect(registry.removeOverlayEntry('app', 'org_authored')).toBe(true);
+        expect(registry.getItem('app', 'org_authored')).toBeUndefined();
+    });
+
+    it('REFUSES a plain-key entry that is itself a packaged artifact', () => {
+        // `loadMetadataFromService` passes the item's own `_packageId` through,
+        // so a package-shipped item can be registered under the plain key.
+        // Unregistering it would delete shipped code an overlay delete never
+        // touched — strictly worse than the staleness this method removes.
+        const registry = new SchemaRegistry({ multiTenant: false });
+        registry.registerItem('app', { name: 'shipped', label: 'Shipped', _packageId: PKG }, 'name');
+
+        expect(registry.removeOverlayEntry('app', 'shipped')).toBe(false);
+        expect((registry.getItem('app', 'shipped') as any)?.label).toBe('Shipped');
+    });
+
+    it('never touches composite keys, and reports nothing to remove', () => {
+        const registry = new SchemaRegistry({ multiTenant: false });
+        registry.registerItem('app', artifactApp(), 'name', PKG);
+
+        // No plain-key entry at all: the artifact must survive untouched.
+        expect(registry.removeOverlayEntry('app', 'setup')).toBe(false);
+        expect((registry.getArtifactItem('app', 'setup') as any)?.label).toBe('Setup');
+        // An unknown type is a no-op, not a throw.
+        expect(registry.removeOverlayEntry('nope', 'setup')).toBe(false);
+    });
+});

packages/objectql/src/registry.ts

@@ -1653,6 +1653,55 @@ export class SchemaRegistry {
     return false;
   }
 
+  /**
+   * [#5079] Remove the PLAIN-KEY entry for `(type, name)` — the slot an
+   * ADR-0005 overlay row hydrates into — and nothing else. The composite
+   * (`<packageId>:<name>`) entries are never touched.
+   *
+   * The other half of {@link removeRuntimeShadow}, for the case that method
+   * deliberately declines. `removeRuntimeShadow` drops the plain key only
+   * when a packaged artifact remains underneath, so the name stays
+   * resolvable; that was the whole story while the plain key could only ever
+   * be an artifact's shadow. #4521 changed it: `saveMetaItem` now writes an
+   * overlay through into the registry, so a runtime-CREATED item (nothing
+   * shipped under that name) lives under the plain key too — and its DELETE
+   * had nothing that would ever remove it. `GET /meta/<type>` kept
+   * enumerating a deleted item and `GET /meta/<type>/<name>` kept serving its
+   * body for the life of the process (#4432's "every surface in agreement"
+   * clause, residual). plugin-security's `permission` projection carries a
+   * consumer-side work-around for the same lingering entry
+   * (`readDeclaredBody` skipping shadows so a deleted set is not undeletable);
+   * this is the producer-side removal it was compensating for.
+   *
+   * Whether the item really is gone from every OTHER layer is not a fact this
+   * registry holds — the MetadataService may still serve a baseline for the
+   * name — so the caller decides. `deleteMetaItem`'s
+   * `restoreArtifactRegistryView` calls this only after both lower layers
+   * (composite artifact, MetadataService baseline) have answered "nothing".
+   *
+   * Refuses exactly one entry: a plain-key registration that IS a packaged
+   * artifact — `_packageId` set, not the `'sys_metadata'` rehydration
+   * sentinel, not tenant-authored — which is the same predicate
+   * {@link getArtifactItem} applies to its own bare-key fallback. Artifact
+   * loaders normally register under a composite key, but
+   * `loadMetadataFromService` passes the item's own `_packageId` through, so a
+   * package-stamped item can land here; unregistering shipped code that the
+   * overlay delete never touched would be a worse bug than the one this fixes.
+   *
+   * @returns whether an entry was removed.
+   */
+  removeOverlayEntry(type: string, name: string): boolean {
+    const collection = this.metadata.get(type);
+    if (!collection || !collection.has(name)) return false;
+    const plain = collection.get(name) as any;
+    if (plain && plain._packageId && plain._packageId !== 'sys_metadata' && !isTenantAuthored(plain)) {
+      return false;
+    }
+    collection.delete(name);
+    this.log(`[Registry] Removed overlay entry ${type}: ${name} (no layer serves it any more)`);
+    return true;
+  }
+
   /**
    * Universal List Method
    */