Skip to content

Commit 114e727

Browse files
os-zhuangclaude
andauthored
fix(objectql,metadata-protocol): retire an overlay's registry entry on delete (#5079) (#6687)
Deleting a runtime-CREATED metadata item removed its `sys_metadata` row and reported `reset: true`, while `GET /meta/<type>`, `GET /meta/<type>/<name>` and the ADR-0110 D3 declaration gate all kept serving it for the life of the process. #4521's write-through registers such an item under the SchemaRegistry's PLAIN key; the delete's heal (`removeRuntimeShadow`) only un-shadows a packaged artifact, and a runtime-created item has none — so nothing ever removed it. `restoreArtifactRegistryView` now walks the layers under the deleted overlay and stops at the first that can serve the name: composite-key artifact, then a MetadataService baseline, then — new — retire the plain-key entry through `SchemaRegistry.removeOverlayEntry`. The layer-2 read is diagnosed, so a degraded metadata plane stops the walk instead of retiring on an unread answer. Claude-Session: https://claude.ai/code/session_01W6bLax4KMrSfnE1ydFU8Dw Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
1 parent c6b05c7 commit 114e727

5 files changed

Lines changed: 413 additions & 21 deletions

File tree

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

‎packages/metadata-protocol/src/protocol.ts‎

Lines changed: 57 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -7613,7 +7613,8 @@ export class ObjectStackProtocolImplementation implements
76137613

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

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

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -322,3 +322,67 @@ describe('SchemaRegistry.getArtifactItem / removeRuntimeShadow', () => {
322322
expect((registry.getItem('app', 'mine') as any)?.label).toBe('Mine');
323323
});
324324
});
325+
326+
/**
327+
* [#5079] The other half of the reset heal — the case
328+
* {@link SchemaRegistry.removeRuntimeShadow} above deliberately declines.
329+
*
330+
* A runtime-CREATED item has no packaged artifact under a composite key, so
331+
* `removeRuntimeShadow` leaves its plain-key entry standing (pinned directly
332+
* above, and correct: that method's job is un-shadowing an artifact). Since
333+
* #4521's write-through puts such an item in the registry, nothing else ever
334+
* removed it — `getMetaItems` kept enumerating a deleted item for the life of
335+
* the process. `removeOverlayEntry` is what `deleteMetaItem` calls once both
336+
* lower layers have answered "nothing".
337+
*/
338+
describe('SchemaRegistry.removeOverlayEntry', () => {
339+
it('removes the plain-key entry of a runtime-only item', () => {
340+
const registry = new SchemaRegistry({ multiTenant: false });
341+
registry.registerItem('app', { name: 'mine', label: 'Mine' }, 'name');
342+
343+
expect(registry.removeOverlayEntry('app', 'mine')).toBe(true);
344+
expect(registry.getItem('app', 'mine')).toBeUndefined();
345+
expect(registry.listItems('app')).toEqual([]);
346+
});
347+
348+
it('removes a `sys_metadata`-sentinel rehydration entry', () => {
349+
// `loadMetaFromDb` stamps the sentinel on package-less overlay rows;
350+
// it marks the entry as a rehydration, not a shipped artifact.
351+
const registry = new SchemaRegistry({ multiTenant: false });
352+
registry.registerItem('app', { name: 'hydrated', label: 'Hydrated', _packageId: 'sys_metadata' }, 'name');
353+
354+
expect(registry.removeOverlayEntry('app', 'hydrated')).toBe(true);
355+
expect(registry.getItem('app', 'hydrated')).toBeUndefined();
356+
});
357+
358+
it('removes a tenant-authored entry even when it carries a real package id', () => {
359+
const registry = new SchemaRegistry({ multiTenant: false });
360+
registry.registerItem('app', { name: 'org_authored', label: 'Org', _packageId: PKG, _provenance: 'org' }, 'name');
361+
362+
expect(registry.removeOverlayEntry('app', 'org_authored')).toBe(true);
363+
expect(registry.getItem('app', 'org_authored')).toBeUndefined();
364+
});
365+
366+
it('REFUSES a plain-key entry that is itself a packaged artifact', () => {
367+
// `loadMetadataFromService` passes the item's own `_packageId` through,
368+
// so a package-shipped item can be registered under the plain key.
369+
// Unregistering it would delete shipped code an overlay delete never
370+
// touched — strictly worse than the staleness this method removes.
371+
const registry = new SchemaRegistry({ multiTenant: false });
372+
registry.registerItem('app', { name: 'shipped', label: 'Shipped', _packageId: PKG }, 'name');
373+
374+
expect(registry.removeOverlayEntry('app', 'shipped')).toBe(false);
375+
expect((registry.getItem('app', 'shipped') as any)?.label).toBe('Shipped');
376+
});
377+
378+
it('never touches composite keys, and reports nothing to remove', () => {
379+
const registry = new SchemaRegistry({ multiTenant: false });
380+
registry.registerItem('app', artifactApp(), 'name', PKG);
381+
382+
// No plain-key entry at all: the artifact must survive untouched.
383+
expect(registry.removeOverlayEntry('app', 'setup')).toBe(false);
384+
expect((registry.getArtifactItem('app', 'setup') as any)?.label).toBe('Setup');
385+
// An unknown type is a no-op, not a throw.
386+
expect(registry.removeOverlayEntry('nope', 'setup')).toBe(false);
387+
});
388+
});

‎packages/objectql/src/registry.ts‎

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1653,6 +1653,55 @@ export class SchemaRegistry {
16531653
return false;
16541654
}
16551655

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

0 commit comments

Comments
 (0)