Skip to content

Commit 47a4e67

Browse files
os-zhuangclaude
andauthored
fix(objectql): deleting an object really unregisters it — a name-addressed SchemaRegistry.unregisterObject (#6808) (#6818)
* fix(objectql): deleting an object really unregisters it — name-addressed SchemaRegistry.unregisterObject (#6808) `deleteMetaItem`'s registry heal (`restoreArtifactRegistryView`, the #6687 three-tier walk) addresses only `SchemaRegistry`'s generic `metadata` map, but an `object` is written into two places — that map AND `objectContributors`. The heal undid the first, so after a delete the row was gone while `registry.getObject()` — the surface data CRUD dispatches on — kept serving the object, keeping it readable and writable for the life of the process. No one-line fix existed: the registry's only removal verb was the package-scoped `unregisterObjectsByPackage`. This adds the missing name-addressed verb, `SchemaRegistry.unregisterObject(name, { force })`, honouring ADR-0029's single-owner/extender rules by mirroring the judgement the package-scoped verb already encodes, and calls it from the heal's tier-3 branch only — the tier that has established no lower layer serves the name. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Lewjj7ukQkQ5WT6cJjpdHF * fix(metadata-protocol): the heal's object limb never retires a code-shipped object (#6808) Self-review of the tier-3 removal found a hole in it: on a control-plane kernel the two-tier delete authorization (`environmentId !== undefined`) does not run, and the no-row leg of `deleteMetaItem` reaches the registry heal without touching the repository's `assertAllowed` either — so the walk could arrive for a name a code package still ships and unregister it, taking that object off the whole data plane until restart (`assertObjectRegistered` fails closed). The limb now carries the same artifact refusal `removeOverlayEntry` applies one line up, asked through the protocol's own `isArtifactBacked` rather than a second open-coded predicate. Pinned on both sides, with the tenant-kernel `NOT_OVERRIDABLE` refusal pinned as its pair so neither gate can quietly disappear. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Lewjj7ukQkQ5WT6cJjpdHF --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 53aeb02 commit 47a4e67

7 files changed

Lines changed: 1273 additions & 8 deletions
Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,75 @@
1+
---
2+
"@objectstack/objectql": patch
3+
"@objectstack/metadata-protocol": patch
4+
---
5+
6+
fix(objectql): deleting an `object` really unregisters it — a name-addressed `SchemaRegistry.unregisterObject` (#6808)
7+
8+
Deleting a runtime-created `object` removed its `sys_metadata` row and left the
9+
object serving. `deleteMetaItem` ends its repository delete with
10+
`restoreArtifactRegistryView` (the #6687 three-tier heal), and every verb that
11+
walk uses — `removeRuntimeShadow`, `registerItem`, `removeOverlayEntry`
12+
addresses `SchemaRegistry`'s generic `metadata` map. An `object` is written into
13+
**two** places on the way in:
14+
15+
```ts
16+
registry.registerItem('object', item, 'name'); // metadata map
17+
registry.registerObject({ ...item, _provenance: 'org' }, pkg); // objectContributors
18+
```
19+
20+
The heal only undid the first. Measured with the real `SysMetadataRepository`
21+
over an in-memory engine:
22+
23+
```
24+
BEFORE delete: metadata['object'] -> ["myapp_invoice"] | objectContributors -> ["myapp_invoice"]
25+
AFTER delete: metadata['object'] -> [] | objectContributors -> ["myapp_invoice"]
26+
registry.getObject('myapp_invoice') -> STILL SERVED
27+
registry.getItem('object','myapp_invoice') -> STILL SERVED (it special-cases back to getObject)
28+
```
29+
30+
The surviving half is the load-bearing one. `getObject` is what the data plane
31+
dispatches on (`assertObjectRegistered`, #3770), so the row was gone from
32+
`sys_metadata` while the object stayed resolvable, syncable and **writable** for
33+
the life of the process — a `createData` against the deleted object still
34+
inserted rows. Reachable on the ordinary Studio delete path, and on
35+
`revertCommit`'s soft-remove limb, which #6807 had just wired to the same heal.
36+
37+
There was no one-line fix because `SchemaRegistry` had no per-name object
38+
removal at all: the only removal verb was `unregisterObjectsByPackage`, which is
39+
addressed by PACKAGE. Routing a single delete through it would mean synthesising
40+
a package identity for a runtime-created object and tearing down every sibling
41+
object registered under it — a far wider blast radius than the delete the
42+
operator asked for.
43+
44+
So `SchemaRegistry` gains the verb that was missing:
45+
46+
- **`unregisterObject(name, { force? })`** — removes one object's contributor
47+
entry and the per-object state `registerObject` created (merged-object cache,
48+
`objectRevision`). Names resolve through the same path `getObject` uses, so it
49+
removes precisely the entry that was being served. Package namespaces are left
50+
alone: they are per-package and shared by every object that package ships.
51+
- **The ADR-0029 guard is borrowed, not re-invented.** An object still extended
52+
by another package refuses loudly, naming every extender — the same judgement
53+
`unregisterObjectsByPackage(force)` already encodes, with the address changed
54+
from package to name. Both facts it needs (owner, extenders) were already in
55+
the contributor list, so no new bookkeeping was added.
56+
57+
`restoreArtifactRegistryView` calls it from **tier 3 only**, and only for a name
58+
that is not artifact-backed — the tier that has already established no lower
59+
layer serves the name. Tiers 1 and 2 concluded a
60+
packaged artifact or a MetadataService baseline still does, and an object that is
61+
still served must stay registered: `assertObjectRegistered` fails CLOSED, so
62+
retiring it there would turn "reset to artifact default" into a data-plane
63+
outage. It also carries the same artifact refusal `removeOverlayEntry` applies
64+
one line up, asked through the protocol's own `isArtifactBacked`: a code-shipped
65+
object is never retired by this walk. That is not already covered by the gates in
66+
front of it — the two-tier delete authorization runs only when `environmentId !==
67+
undefined`, and the no-row leg of a control-plane delete reaches the heal without
68+
touching the repository's `assertAllowed` at all.
69+
70+
Because the heal runs after the repository delete has committed, an extender
71+
refusal is caught and logged by name rather than propagated (the row is gone
72+
either way) — and deliberately not left to the heal's silent outer `catch`, so a
73+
runtime that disagrees with `sys_metadata` is visible rather than inferred.
74+
75+
`unregisterObjectsByPackage` keeps its signature and semantics unchanged.
Lines changed: 301 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,301 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* #6808 — the registry heal's OBJECT limb, pinned at this package's own seam.
5+
*
6+
* `deleteMetaItem` ends its repository delete with `restoreArtifactRegistryView`
7+
* (the #6687 three-tier walk). Every verb that walk used —
8+
* `removeRuntimeShadow`, `registerItem`, `removeOverlayEntry` — addresses
9+
* `SchemaRegistry`'s generic `metadata` map. An `object` is written into TWO
10+
* places on the way in (`registerItem` into that map, `registerObject` into
11+
* `objectContributors`), so the walk retired the LISTING copy and left the
12+
* DISPATCH copy: `registry.getObject(name)` kept serving a deleted object, and
13+
* with it every data-plane write, because `assertObjectRegistered` reads
14+
* exactly that.
15+
*
16+
* This file pins the CONTRACT between the two packages — that the heal calls a
17+
* name-addressed object removal, on the right tier and for the right type, and
18+
* degrades rather than throwing when the registry in hand does not have one.
19+
* The behavioural half (a real `SchemaRegistry`, real repository, both exits
20+
* measured, data CRUD refused afterwards) lives in `@objectstack/objectql`,
21+
* which is where the registry lives:
22+
* `protocol-delete-object-registry-heal.test.ts`.
23+
*
24+
* ---------------------------------------------------------------------------
25+
* Reverse verification, direction predicted BEFORE running
26+
* ---------------------------------------------------------------------------
27+
* Removing the `registry.unregisterObject(name)` call from the heal turns the
28+
* two "calls it" cases in this file red on the call count (`[]` vs
29+
* `['rc9_widget']`) and leaves every negative case green — they assert the call
30+
* is NOT made, which is trivially true with no call site at all. That asymmetry
31+
* is intended: the negatives constrain the SHAPE of the fix (tier and type),
32+
* not its presence.
33+
*/
34+
import { describe, expect, it, vi } from 'vitest';
35+
// [#5619] The producer's OWN write-verb dispatch decisions (#4550 delete /
36+
// #5480 update). Imported from `@objectstack/metadata-core`, never from
37+
// `@objectstack/objectql`: objectql DEPENDS ON this package, so that import
38+
// would close a dependency cycle turbo rejects outright.
39+
import { assertEngineDeleteDispatch, assertEngineUpdateDispatch } from '@objectstack/metadata-core';
40+
import { ObjectStackProtocolImplementation } from './protocol.js';
41+
42+
interface Row {
43+
id: string;
44+
type: string;
45+
name: string;
46+
organization_id: string | null;
47+
state: string;
48+
checksum: string;
49+
metadata: string;
50+
}
51+
52+
/** An overlay row as `deleteMetaItem` finds it — a real `checksum` or the OCC */
53+
/** parent-version check 409s before the heal is ever reached. */
54+
const overlayRow = (type: string, name: string): Row => ({
55+
id: `row_${type}_${name}`,
56+
type,
57+
name,
58+
organization_id: null,
59+
state: 'active',
60+
checksum: 'sha256:stored-head',
61+
metadata: JSON.stringify({ name, label: 'Stored' }),
62+
});
63+
64+
function makeHarness(opts: {
65+
rows?: Row[];
66+
/** Tier 1 answers "a packaged artifact is underneath" for these `type|name`s. */
67+
shadowed?: Array<{ type: string; name: string }>;
68+
/** Simulate an older registry double that has no name-addressed removal. */
69+
omitUnregisterObject?: boolean;
70+
/** The ADR-0029 refusal, raised by the registry the way the real verb does. */
71+
unregisterThrows?: Error;
72+
/**
73+
* ADR-0010 `_provenance` of what the registry currently serves for the
74+
* name. `'org'` is what BOTH paths that register a tenant's object stamp
75+
* (the write-through and the boot rehydration); `'package'` is a
76+
* loader-introduced artifact.
77+
*/
78+
servedProvenance?: 'org' | 'package' | null;
79+
/** `environmentId === undefined` — the kernel on which the two-tier delete
80+
* authorization does not run. A FLAG, never an `environmentId` parameter
81+
* with a default: passing `undefined` explicitly to a defaulted parameter
82+
* re-applies the default (#6621). */
83+
controlPlane?: boolean;
84+
} = {}) {
85+
const rows = [...(opts.rows ?? [])];
86+
const shadowKeys = new Set((opts.shadowed ?? []).map((s) => `${s.type}|${s.name}`));
87+
const unregisterObjectCalls: string[] = [];
88+
const removeOverlayEntryCalls: string[] = [];
89+
const matches = (row: Row, where: Record<string, unknown> = {}) =>
90+
Object.entries(where).every(([k, v]) => (row as any)[k] === v);
91+
92+
const provenance = opts.servedProvenance === undefined ? 'org' : opts.servedProvenance;
93+
const registry: any = {
94+
getObject: (name: string) =>
95+
provenance === null ? undefined : { name, _packageId: 'app.myapp', _provenance: provenance },
96+
getItem: () => undefined,
97+
listItems: () => [],
98+
registerItem: () => {},
99+
registerObject: () => {},
100+
applyNavContributions: (x: unknown) => x,
101+
isPackageDisabled: () => false,
102+
getObjectOwner: () => undefined,
103+
// Mirrors the REAL `SchemaRegistry.getArtifactItem` for `object`: it
104+
// reads `getObject` and returns it only when it looks like packaged
105+
// code (`_packageId` real, and NOT `_provenance: 'org'`). A double that
106+
// always answered `undefined` here would report every object as
107+
// runtime-authored and hide the artifact refusal entirely — looser than
108+
// the implementation it stands in for, which is no test at all (#4550).
109+
getArtifactItem: (type: string, name: string) => {
110+
if (type !== 'object' && type !== 'objects') return undefined;
111+
const obj: any = registry.getObject(name);
112+
return obj && obj._packageId && obj._packageId !== 'sys_metadata' && obj._provenance !== 'org'
113+
? obj
114+
: undefined;
115+
},
116+
removeRuntimeShadow: (type: string, name: string) => shadowKeys.has(`${type}|${name}`),
117+
removeOverlayEntry: (type: string, name: string) => {
118+
removeOverlayEntryCalls.push(`${type}|${name}`);
119+
return true;
120+
},
121+
};
122+
if (!opts.omitUnregisterObject) {
123+
registry.unregisterObject = (name: string) => {
124+
unregisterObjectCalls.push(name);
125+
if (opts.unregisterThrows) throw opts.unregisterThrows;
126+
return true;
127+
};
128+
}
129+
130+
const engine: any = {
131+
async findOne(table: string, query: { where?: Record<string, unknown> } = {}) {
132+
if (table !== 'sys_metadata') return null;
133+
return rows.find((r) => matches(r, query.where)) ?? null;
134+
},
135+
async find() { return []; },
136+
async insert(_table: string, data: Record<string, unknown>) { return { id: 'inserted', ...data }; },
137+
async update(_table: string, data: Record<string, unknown>, options?: Record<string, unknown>) {
138+
assertEngineUpdateDispatch(data, options);
139+
return { id: null };
140+
},
141+
async delete(_table: string, options?: Record<string, unknown>) {
142+
assertEngineDeleteDispatch(options);
143+
const id = (options as any)?.where?.id;
144+
const at = rows.findIndex((r) => r.id === id);
145+
if (at >= 0) rows.splice(at, 1);
146+
return { deleted: at >= 0 ? 1 : 0 };
147+
},
148+
async count() { return 0; },
149+
async transaction(fn: (ctx: unknown) => Promise<unknown>) { return fn(undefined); },
150+
async execute() { return {}; },
151+
async getObjectSchema() { return undefined; },
152+
async syncObjectSchema() { /* no physical storage in this double */ },
153+
registry,
154+
};
155+
156+
// An EMPTY services registry: with no `metadata` service, tier 2 answers
157+
// "no baseline, not degraded", which is the verdict that licenses tier 3.
158+
const protocol = new ObjectStackProtocolImplementation(
159+
engine, () => new Map(), opts.controlPlane === true ? undefined : 'env_1',
160+
) as any;
161+
return { protocol, rows, unregisterObjectCalls, removeOverlayEntryCalls };
162+
}
163+
164+
describe('#6808 — the heal retires the object contributor, not just the metadata entry', () => {
165+
it('a deleted runtime-only object is unregistered BY NAME', async () => {
166+
const h = makeHarness({ rows: [overlayRow('object', 'rc9_widget')] });
167+
168+
const result = await h.protocol.deleteMetaItem({ type: 'object', name: 'rc9_widget' });
169+
170+
expect(result.success).toBe(true);
171+
expect(h.rows).toHaveLength(0);
172+
// Pre-fix: `[]`. The generic-map half ran and the contributor half did not.
173+
expect(h.unregisterObjectCalls).toEqual(['rc9_widget']);
174+
// …alongside the half that always ran, not instead of it.
175+
expect(h.removeOverlayEntryCalls).toContain('object|rc9_widget');
176+
});
177+
178+
it('the plural `objects` spelling reaches the same limb, once', async () => {
179+
const h = makeHarness({ rows: [overlayRow('object', 'rc9_widget')] });
180+
181+
await h.protocol.deleteMetaItem({ type: 'objects', name: 'rc9_widget' });
182+
183+
// Keyed off the SINGULAR, so the twin spelling neither misses it nor
184+
// doubles it (#4432 — every surface in agreement).
185+
expect(h.unregisterObjectCalls).toEqual(['rc9_widget']);
186+
});
187+
188+
it('a NON-object type never reaches it — only `object` has a second home', async () => {
189+
const h = makeHarness({ rows: [overlayRow('view', 'rc9_grid')] });
190+
191+
const result = await h.protocol.deleteMetaItem({ type: 'view', name: 'rc9_grid' });
192+
193+
expect(result.success).toBe(true);
194+
expect(h.removeOverlayEntryCalls).toContain('view|rc9_grid');
195+
expect(h.unregisterObjectCalls).toEqual([]);
196+
});
197+
198+
/**
199+
* The tier discipline. Tier 1 concluded a packaged artifact still serves the
200+
* name, so the walk returns before tier 3 — and an object that is still
201+
* served must stay registered: `assertObjectRegistered` fails CLOSED, so
202+
* retiring it here would turn "reset to artifact default" into a data-plane
203+
* outage for a name a code package still ships.
204+
*/
205+
it('an object whose overlay shadows an artifact is NOT unregistered (tier 1 stops the walk)', async () => {
206+
const h = makeHarness({
207+
rows: [overlayRow('object', 'rc9_widget')],
208+
shadowed: [{ type: 'object', name: 'rc9_widget' }],
209+
});
210+
211+
const result = await h.protocol.deleteMetaItem({ type: 'object', name: 'rc9_widget' });
212+
213+
expect(result.success).toBe(true);
214+
expect(h.rows).toHaveLength(0);
215+
expect(h.removeOverlayEntryCalls).toEqual([]);
216+
expect(h.unregisterObjectCalls).toEqual([]);
217+
});
218+
219+
/**
220+
* The second refusal, and the one that is NOT theoretical for objects:
221+
* `engine.registerApp` registers a package's objects straight into
222+
* `objectContributors` without writing the generic `metadata` map, so
223+
* `isArtifactBacked` does not see them and an overlay row for such a name
224+
* CAN be authored. Retiring the contributor on that row's delete would take
225+
* a code package's object off the data plane until restart.
226+
*
227+
* The axis is ADR-0010 `_provenance`, the same one `removeOverlayEntry`
228+
* uses one line up — both paths that register a TENANT's object stamp
229+
* `'org'` server-side, an artifact carries `'package'`.
230+
*/
231+
it('a CODE-SHIPPED object is never unregistered by this walk', async () => {
232+
// The reachable shape: a CONTROL-PLANE kernel (the two-tier delete
233+
// authorization that refuses an artifact-backed `object` with
234+
// `NOT_OVERRIDABLE` is wrapped in `environmentId !== undefined`), on the
235+
// NO-ROW leg — which runs the heal without ever reaching the
236+
// repository's own `assertAllowed`. `revertCommit`'s soft-remove limb
237+
// reaches the walk without that gate either.
238+
const h = makeHarness({ controlPlane: true, servedProvenance: 'package' });
239+
240+
const result = await h.protocol.deleteMetaItem({ type: 'object', name: 'rc9_widget' });
241+
242+
expect(result.success).toBe(true);
243+
// The generic-map half still runs — that entry IS the overlay's slot,
244+
// and `removeOverlayEntry` applies its own artifact refusal inside.
245+
expect(h.removeOverlayEntryCalls).toContain('object|rc9_widget');
246+
expect(h.unregisterObjectCalls).toEqual([]);
247+
});
248+
249+
/**
250+
* The ADR-0029 refusal, at THIS seam. The registry verb throws when an
251+
* extender still depends on the owner; the heal runs after the repository
252+
* delete has committed, so it must not propagate that — the row is gone
253+
* either way and a throw here would turn a successful delete into a 500.
254+
* What it must not do is swallow it into the silent outer `catch`: a runtime
255+
* that disagrees with `sys_metadata` has to be visible in the log.
256+
*/
257+
it('an extender refusal is stated, not swallowed — and the delete still succeeds', async () => {
258+
const h = makeHarness({
259+
rows: [overlayRow('object', 'rc9_widget')],
260+
unregisterThrows: new Error(
261+
'Cannot unregister object "rc9_widget": it is extended by app.addon. Unregister the extenders first.',
262+
),
263+
});
264+
const warn = vi.spyOn(console, 'warn').mockImplementation(() => {});
265+
let result: any;
266+
let warned: string[];
267+
try {
268+
result = await h.protocol.deleteMetaItem({ type: 'object', name: 'rc9_widget' });
269+
} finally {
270+
// Read BEFORE restoring — `mockRestore` also resets recorded calls.
271+
warned = warn.mock.calls.map((c) => String(c[0]));
272+
warn.mockRestore();
273+
}
274+
275+
expect(result.success).toBe(true);
276+
expect(h.rows).toHaveLength(0);
277+
const refusals = warned.filter((m) => m.includes('stays registered'));
278+
expect(refusals).toHaveLength(1);
279+
expect(refusals[0]).toContain('rc9_widget');
280+
expect(refusals[0]).toContain('app.addon');
281+
});
282+
283+
/**
284+
* The same `typeof … === 'function'` courtesy every other verb in this walk
285+
* extends to a partial registry double (edge/Lite embeddings, engine
286+
* doubles). A missing verb is a no-op, never a crash that would strand the
287+
* rest of the delete.
288+
*/
289+
it('a registry without the verb degrades quietly — the delete still succeeds', async () => {
290+
const h = makeHarness({
291+
rows: [overlayRow('object', 'rc9_widget')],
292+
omitUnregisterObject: true,
293+
});
294+
295+
const result = await h.protocol.deleteMetaItem({ type: 'object', name: 'rc9_widget' });
296+
297+
expect(result.success).toBe(true);
298+
expect(h.rows).toHaveLength(0);
299+
expect(h.removeOverlayEntryCalls).toContain('object|rc9_widget');
300+
});
301+
});

0 commit comments

Comments
 (0)