Skip to content

Commit 3eea67e

Browse files
committed
fix(metadata-protocol): unscoped metadata list dedupes package-aware, not by bare name (ADR-0048 #1828)
`getMetaItems` merged registry items, `sys_metadata` overlay rows, draft-preview rows, and MetadataService items into `Map`s keyed by bare `name`. When two installed packages ship the same `type/name` (e.g. `page/home`) and either has an overlay, the DB-overlay merge collapsed them to one row (last-write-wins), so an unscoped `GET /meta/:type` returned only one — and the frontend prefer-local resolution, which reads the unscoped list, could no longer tell the two packages' rows apart. Key the three merge sites (plus the env/org `mergedMap`) by `(package, name)` via a shared `mergePackageAwareOverlay` helper whose per-package resolution mirrors `getMetaItem(name, packageId=P)` (scoped row, else package-less/global fallback), so: - two packages' same-name rows stay distinct, each with its own `_packageId`; - a package-less (env-wide) overlay still wins over the single artifact it customizes — ADR-0005 precedence and single-package behaviour unchanged; - a legacy row whose active/draft layers disagree on package attribution still collapses (draft wins), matching the single-item path. Also scope the registry-hydration artifact graft to each row's own `package_id` so a colliding overlay no longer grafts the first-registered package's provenance/lock onto another package's row (the secondary provenance-pollution gap in #1828). Tests: two-package overlay collision keeps both rows with correct `_packageId`; single-package env-wide overlay unchanged; MetadataService collision keeps both. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DKFjwN2EVXPB2CJwNzPs8c
1 parent 06cb319 commit 3eea67e

3 files changed

Lines changed: 253 additions & 65 deletions

File tree

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
---
2+
'@objectstack/metadata-protocol': patch
3+
---
4+
5+
fix(metadata-protocol): unscoped metadata list dedupes package-aware, not by bare name (ADR-0048 #1828)
6+
7+
`getMetaItems` merged registry items, `sys_metadata` overlay rows, draft-preview
8+
rows, and MetadataService items into `Map`s keyed by bare `name`, so two installed
9+
packages shipping the same `type/name` (e.g. `page/home`) collapsed to one row
10+
(last-write-wins) on an unscoped `GET /meta/:type` whenever either package had an
11+
overlay — and the frontend prefer-local resolution, which reads that list, could
12+
no longer tell the two packages' rows apart.
13+
14+
The three merge sites (plus the env/org pre-merge) now key by `(package, name)`,
15+
mirroring `getMetaItem`'s scoped-then-global-fallback resolution: colliding rows
16+
stay distinct each with its own `_packageId`, a package-less (env-wide) overlay
17+
still wins over the single artifact it customizes (ADR-0005 precedence and
18+
single-package behaviour unchanged), and the registry-hydration artifact graft is
19+
scoped to each row's own `package_id` so a collision no longer mislabels provenance.

packages/metadata-protocol/src/protocol.ts

Lines changed: 173 additions & 65 deletions
Original file line numberDiff line numberDiff line change
@@ -369,6 +369,104 @@ function mergeArtifactProtection(item: unknown, artifactItem: unknown): unknown
369369
return out;
370370
}
371371

372+
/**
373+
* ADR-0048 (#1828) — composite dedup identity for the unscoped metadata list.
374+
*
375+
* Two installed packages may legitimately ship the same `type`/`name`
376+
* (e.g. `page/home`); the SchemaRegistry already stores them under distinct
377+
* `${packageId}:${name}` keys. Any list-merge that deduplicates by bare `name`
378+
* collapses the two packages' rows into one (last-write-wins), which is the
379+
* bug this key closes. A `NUL` separator keeps names containing `:` unambiguous.
380+
*/
381+
function metaItemKey(packageId: string | null | undefined, name: unknown): string {
382+
return `${packageId ?? ''}\u0000${String(name)}`;
383+
}
384+
385+
/**
386+
* ADR-0048 (#1828) — package-aware overlay merge for the unscoped metadata list.
387+
*
388+
* `baseItems` (the lower layer: registry artifacts, or the running result) and
389+
* `records` (the higher layer: active `sys_metadata` overlays, or draft rows)
390+
* are merged so that:
391+
*
392+
* • Two installed packages shipping the same `type/name` stay TWO rows —
393+
* resolution is per `(package, name)`, not bare `name`, so a higher-layer
394+
* row no longer collapses a same-name row from a different package.
395+
* • For each package `P` that owns a row of a given name, the winner is the
396+
* LATEST contribution that is either `P`'s own row or a package-less
397+
* ("global", `package_id IS NULL`) row — mirroring
398+
* `getMetaItem(name, packageId=P)`'s "scoped-then-global-fallback"
399+
* resolution, so the list and single-item paths agree. This is also why a
400+
* legacy row whose active/draft layers disagree on package attribution
401+
* still collapses (a package-less active row + its `package_id`-bearing
402+
* draft resolve to the one package slot, draft winning).
403+
* • A name with NO package-owned row resolves to its latest package-less
404+
* contribution — the pre-existing env-wide behaviour, unchanged.
405+
*
406+
* `transform(data, prev)` runs on each `records` body before it enters the
407+
* merge (view-identity healing, draft tagging); `prev` is the base row it
408+
* shadows at the same slot (or any same-name base row), else undefined.
409+
*/
410+
function mergePackageAwareOverlay(
411+
baseItems: unknown[],
412+
records: Array<{ data: unknown; packageId: string | undefined }>,
413+
transform?: (data: any, prev: any) => any,
414+
): unknown[] {
415+
// Per-name, layer-ordered contributions; `pkg: undefined` = package-less.
416+
const buckets = new Map<string, Array<{ pkg: string | undefined; item: any }>>();
417+
const order: string[] = []; // first-seen name order → stable output
418+
const push = (name: string, pkg: string | undefined, item: any) => {
419+
let list = buckets.get(name);
420+
if (!list) { buckets.set(name, (list = [])); order.push(name); }
421+
list.push({ pkg, item });
422+
};
423+
424+
for (const raw of baseItems) {
425+
const item = raw as any;
426+
if (item && typeof item === 'object' && 'name' in item) {
427+
push(item.name, (item._packageId ?? undefined) as string | undefined, item);
428+
}
429+
}
430+
for (const { data, packageId } of records) {
431+
const body = data as any;
432+
if (!(body && typeof body === 'object' && 'name' in body)) continue;
433+
// The base row this record shadows at its own slot (for view-identity
434+
// healing): a same-package row, else a package-less one, else any
435+
// same-name row it stands in for.
436+
const list = buckets.get(body.name);
437+
const prev = list
438+
? (list.find((c) => c.pkg === packageId)?.item
439+
?? list.find((c) => c.pkg === undefined)?.item
440+
?? list[0]?.item)
441+
: undefined;
442+
push(body.name, packageId, transform ? transform(body, prev) : body);
443+
}
444+
445+
const out: unknown[] = [];
446+
for (const name of order) {
447+
const list = buckets.get(name)!;
448+
const reals = Array.from(new Set(list.filter((c) => c.pkg !== undefined).map((c) => c.pkg)));
449+
if (reals.length === 0) {
450+
out.push(list[list.length - 1].item); // latest package-less row wins
451+
continue;
452+
}
453+
for (const real of reals) {
454+
// getMetaItem(name, real) resolution: latest row that is `real`'s
455+
// own or package-less (global fallback).
456+
let chosen: any;
457+
for (const c of list) {
458+
if (c.pkg === real || c.pkg === undefined) chosen = c.item;
459+
}
460+
if (chosen === undefined) continue;
461+
// A package-less body standing in for package `real` must carry
462+
// `real`'s provenance (the base row it replaced was `real`'s).
463+
if (chosen._packageId === undefined) chosen = { ...chosen, _packageId: real };
464+
out.push(chosen);
465+
}
466+
}
467+
return out;
468+
}
469+
372470
/**
373471
* Simple hash function for ETag generation (browser-compatible)
374472
* Uses a basic hash algorithm instead of crypto.createHash
@@ -1608,63 +1706,69 @@ export class ObjectStackProtocolImplementation implements ObjectStackProtocol {
16081706
};
16091707
const envWideRecords = await queryByOrg(null);
16101708
const orgRecords = orgId ? await queryByOrg(orgId) : [];
1611-
// org-specific rows override env-wide rows on name collision
1709+
// org-specific rows override env-wide rows on name collision.
1710+
// ADR-0048 (#1828) — key by (package, name), not bare name, so a
1711+
// package A row and a package B row of the same name do not
1712+
// collapse; org-over-env precedence still holds within each slot.
16121713
const mergedMap = new Map<string, any>();
1613-
for (const r of envWideRecords) mergedMap.set(r.name, r);
1614-
for (const r of orgRecords) mergedMap.set(r.name, r);
1714+
for (const r of envWideRecords) mergedMap.set(metaItemKey(r.package_id, r.name), r);
1715+
for (const r of orgRecords) mergedMap.set(metaItemKey(r.package_id, r.name), r);
16151716
const records = Array.from(mergedMap.values());
16161717
if (records && records.length > 0) {
1617-
const byName = new Map<string, any>();
1618-
for (const existing of items) {
1619-
const entry = existing as any;
1620-
if (entry && typeof entry === 'object' && 'name' in entry) {
1621-
byName.set(entry.name, entry);
1622-
}
1623-
}
1624-
for (const record of records) {
1718+
const isView = (PLURAL_TO_SINGULAR[request.type] ?? request.type) === 'view';
1719+
// Parse each overlay body once and surface its persisted
1720+
// software-package binding so the sidebar package filter and
1721+
// provenance classification see overlay rows the way they see
1722+
// registry items.
1723+
const overlays = records.map((record) => {
16251724
const data = typeof record.metadata === 'string'
16261725
? JSON.parse(record.metadata)
16271726
: record.metadata;
1628-
if (data && typeof data === 'object' && 'name' in data) {
1629-
// Surface the persisted software-package binding so the
1630-
// sidebar package filter and provenance classification
1631-
// see overlay rows the same way they see registry items.
1632-
const recPkg = (record as { package_id?: string | null }).package_id ?? undefined;
1633-
if (recPkg && (data as any)._packageId === undefined) {
1634-
(data as any)._packageId = recPkg;
1635-
}
1636-
// #2555 — heal identity-less view overlays already in the
1637-
// DB (persisted by pre-fix saves): a raw-config row would
1638-
// replace the flattened package entry wholesale, dropping
1639-
// viewKind/object and vanishing the view from every
1640-
// consumer that filters on them (switcher endpoint).
1641-
// Inherit the identity fields from the shadowed entry;
1642-
// the overlay's own fields still win.
1643-
if ((PLURAL_TO_SINGULAR[request.type] ?? request.type) === 'view') {
1644-
const patch = viewIdentityPatch(data as Record<string, unknown>, byName.get(data.name));
1645-
if (patch) Object.assign(data, patch);
1646-
}
1647-
byName.set(data.name, data);
1727+
const recPkg = (record as { package_id?: string | null }).package_id ?? undefined;
1728+
if (recPkg && data && typeof data === 'object' && (data as any)._packageId === undefined) {
1729+
(data as any)._packageId = recPkg;
16481730
}
1649-
// Only hydrate the global registry for unscoped calls —
1650-
// scoped project entries must not leak process-wide.
1651-
// Graft the artifact's protection envelope onto the
1652-
// overlay body BEFORE registering: the plain-key entry
1653-
// written here shadows the packaged artifact on
1654-
// `registry.getItem`, and a bare overlay body would
1655-
// strip `_lock`/`_packageId`/`_provenance` from every
1656-
// registry-direct reader (ADR-0010 §3.3 — an overlay
1657-
// must never loosen a packaged lock).
1658-
if (this.environmentId === undefined && data && typeof data === 'object') {
1659-
const artifact = this.lookupArtifactItem(request.type, (data as any).name);
1660-
this.engine.registry.registerItem(
1661-
request.type,
1662-
mergeArtifactProtection(data, artifact),
1663-
'name' as any,
1664-
);
1731+
return { data, packageId: recPkg };
1732+
});
1733+
1734+
// ADR-0048 (#1828) — package-aware merge: a package-scoped row
1735+
// overlays ONLY its own package's entry, so two installed
1736+
// packages shipping the same `type/name` (e.g. `page/home`) are
1737+
// not collapsed to one. #2555 — heal identity-less view overlays
1738+
// from the entry they shadow (a raw-config row would otherwise
1739+
// drop viewKind/object and vanish the view from switcher/list
1740+
// consumers); the overlay's own fields still win.
1741+
items = mergePackageAwareOverlay(items, overlays, (data, prev) => {
1742+
if (isView && data && typeof data === 'object') {
1743+
const patch = viewIdentityPatch(data as Record<string, unknown>, prev);
1744+
if (patch) Object.assign(data as Record<string, unknown>, patch);
1745+
}
1746+
return data;
1747+
});
1748+
1749+
// Only hydrate the global registry for unscoped (control-plane)
1750+
// calls — scoped project entries must not leak process-wide.
1751+
// Graft the artifact's protection envelope onto the overlay body
1752+
// BEFORE registering: the plain-key entry written here shadows
1753+
// the packaged artifact on `registry.getItem`, and a bare
1754+
// overlay body would strip `_lock`/`_packageId`/`_provenance`
1755+
// from every registry-direct reader (ADR-0010 §3.3 — an overlay
1756+
// must never loosen a packaged lock). ADR-0048 (#1828) — scope
1757+
// the artifact lookup to the row's OWN package so a colliding
1758+
// overlay no longer grafts the first-registered package's
1759+
// provenance/lock onto another package's row.
1760+
if (this.environmentId === undefined) {
1761+
for (const { data, packageId: recPkg } of overlays) {
1762+
if (data && typeof data === 'object' && 'name' in data) {
1763+
const artifact = this.lookupArtifactItem(request.type, (data as any).name, recPkg);
1764+
this.engine.registry.registerItem(
1765+
request.type,
1766+
mergeArtifactProtection(data, artifact),
1767+
'name' as any,
1768+
);
1769+
}
16651770
}
16661771
}
1667-
items = Array.from(byName.values());
16681772
}
16691773
} catch {
16701774
// DB not available — fall through with whatever we already have.
@@ -1698,21 +1802,22 @@ export class ObjectStackProtocolImplementation implements ObjectStackProtocol {
16981802
};
16991803
const draftRecords = [...(await queryDrafts(null)), ...(orgId ? await queryDrafts(orgId) : [])];
17001804
if (draftRecords.length > 0) {
1701-
const byName = new Map<string, any>();
1702-
for (const existing of items) {
1703-
const entry = existing as any;
1704-
if (entry && typeof entry === 'object' && 'name' in entry) byName.set(entry.name, entry);
1705-
}
1706-
for (const record of draftRecords) {
1805+
// ADR-0048 (#1828) — package-aware draft overlay (parity with
1806+
// the active-overlay merge above): a package-scoped draft
1807+
// previews only its own package's entry, so two packages'
1808+
// same-name drafts stay distinct. Draft rows win over active.
1809+
const drafts = draftRecords.map((record) => {
17071810
const data = typeof record.metadata === 'string' ? JSON.parse(record.metadata) : record.metadata;
1708-
if (data && typeof data === 'object' && 'name' in data) {
1709-
const recPkg = (record as { package_id?: string | null }).package_id ?? undefined;
1710-
if (recPkg && (data as any)._packageId === undefined) (data as any)._packageId = recPkg;
1711-
(data as any)._draft = true;
1712-
byName.set(data.name, data);
1811+
const recPkg = (record as { package_id?: string | null }).package_id ?? undefined;
1812+
if (recPkg && data && typeof data === 'object' && (data as any)._packageId === undefined) {
1813+
(data as any)._packageId = recPkg;
17131814
}
1714-
}
1715-
items = Array.from(byName.values());
1815+
return { data, packageId: recPkg };
1816+
});
1817+
items = mergePackageAwareOverlay(items, drafts, (data) => {
1818+
if (data && typeof data === 'object') (data as any)._draft = true;
1819+
return data;
1820+
});
17161821
}
17171822
} catch {
17181823
// DB unavailable — serve the active result unchanged.
@@ -1733,12 +1838,14 @@ export class ObjectStackProtocolImplementation implements ObjectStackProtocol {
17331838
runtimeItems = runtimeItems.filter((item: any) => item?._packageId === packageId);
17341839
}
17351840
if (runtimeItems && runtimeItems.length > 0) {
1736-
// Merge, avoiding duplicates by name
1841+
// Merge, avoiding duplicates. ADR-0048 (#1828) — key by
1842+
// (package, name), not bare name, so a runtime item from one
1843+
// package does not collapse a same-name item from another.
17371844
const itemMap = new Map<string, any>();
17381845
for (const item of items) {
17391846
const entry = item as any;
17401847
if (entry && typeof entry === 'object' && 'name' in entry) {
1741-
itemMap.set(entry.name, entry);
1848+
itemMap.set(metaItemKey(entry._packageId ?? undefined, entry.name), entry);
17421849
}
17431850
}
17441851
for (const item of runtimeItems) {
@@ -1752,8 +1859,9 @@ export class ObjectStackProtocolImplementation implements ObjectStackProtocol {
17521859
// view overlays disappear from list endpoints on
17531860
// refresh (detail endpoint kept showing the
17541861
// overlay because it uses a different code path).
1755-
if (!itemMap.has(entry.name)) {
1756-
itemMap.set(entry.name, entry);
1862+
const key = metaItemKey(entry._packageId ?? undefined, entry.name);
1863+
if (!itemMap.has(key)) {
1864+
itemMap.set(key, entry);
17571865
}
17581866
}
17591867
}

0 commit comments

Comments
 (0)