From 170d781c864c45ca1f68a16b6ffe6320ffa1729b Mon Sep 17 00:00:00 2001 From: Jack Zhuang <277994282+os-zhuang@users.noreply.github.com> Date: Sat, 13 Jun 2026 02:07:14 +0500 Subject: [PATCH] =?UTF-8?q?feat(objectql):=20ADR-0048=20=E2=80=94=20detect?= =?UTF-8?q?=20cross-package=20metadata=20collisions?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The metadata registry key is org/type/name with no package coordinate. Object names are kernel namespace-prefix-validated (they map to physical tables), but bare-named UI/automation metadata (page, dashboard, flow, app, action, doc, …) is only snake_case-validated. So two installed packages each defining e.g. a `page` named `home` collide on the same logical key, and bare-name read resolution (getItem) silently returns whichever registered first — last-write-wins with no diagnostic. Decision (ADR-0048): detect cross-package base-layer collisions at registration time and raise an explicit, actionable error naming both packages and the type/name — rather than retrofitting namespace-prefix enforcement onto every legacy bare-named type. Prefix stays a recommended convention; new types (e.g. doc) can be prefix-strict from day one. - registry: MetadataCollisionError + isRealPackage (excludes the 'sys_metadata' rehydration sentinel); guard in registerItem that refuses a real packageId registering a (type,name) already owned by a DIFFERENT real package; findOtherPackageOwner scans the live collection (no parallel index to drift). collisionPolicy 'error' (default) | 'warn', env OS_METADATA_COLLISION=warn for migrations. - Legitimate same-key writes pass through untouched: same-package reloads, runtime/DB overlays (ADR-0005), object ownership/extension, nav contributions. - tests: registry-cross-package-collision (unit) + engine-cross-package-collision (e2e through ObjectQL.registerApp). Suites green: objectql 587, metadata-core 80, spec 6542. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../adr-0048-cross-package-collision.md | 9 + .../0048-cross-package-metadata-collision.md | 220 ++++++++++++++++++ .../engine-cross-package-collision.test.ts | 57 +++++ .../registry-cross-package-collision.test.ts | 126 ++++++++++ packages/objectql/src/registry.ts | 131 +++++++++++ 5 files changed, 543 insertions(+) create mode 100644 .changeset/adr-0048-cross-package-collision.md create mode 100644 docs/adr/0048-cross-package-metadata-collision.md create mode 100644 packages/objectql/src/engine-cross-package-collision.test.ts create mode 100644 packages/objectql/src/registry-cross-package-collision.test.ts diff --git a/.changeset/adr-0048-cross-package-collision.md b/.changeset/adr-0048-cross-package-collision.md new file mode 100644 index 0000000000..d3371e4356 --- /dev/null +++ b/.changeset/adr-0048-cross-package-collision.md @@ -0,0 +1,9 @@ +--- +"@objectstack/objectql": minor +--- + +ADR-0048: cross-package metadata collision detection. Bare-named generic metadata (`page`, `dashboard`, `flow`, `app`, `action`, `doc`, …) carries no package coordinate in the registry key (`org/type/name`), so two installed packages defining the same `(type, name)` would silently shadow each other at read time (`getItem` returns whichever the registry iterates first). The kernel only prefix-validates object names, leaving these types unguarded. + +`SchemaRegistry.registerItem` now refuses a cross-package base-layer collision — a real `packageId` registering a `(type, name)` already owned by a *different* real package — with a `MetadataCollisionError` naming both packages and the type/name. `ObjectQL.registerApp` and the nested-plugin loop delegate to it, so manifest and plugin metadata are both covered. + +Legitimate same-key writes are unaffected: same-package reloads, runtime/DB overlays (ADR-0005, bare-key or `sys_metadata`-sentinel rows), object ownership/extension, and nav contributions all pass through. Policy is `error` by default; set `collisionPolicy: 'warn'` (or `OS_METADATA_COLLISION=warn`) to downgrade during a deliberate migration. diff --git a/docs/adr/0048-cross-package-metadata-collision.md b/docs/adr/0048-cross-package-metadata-collision.md new file mode 100644 index 0000000000..98a6104340 --- /dev/null +++ b/docs/adr/0048-cross-package-metadata-collision.md @@ -0,0 +1,220 @@ +# ADR-0048: Cross-package metadata collision — detect, don't silently overwrite + +**Status**: Proposed (2026-06-13) +**Deciders**: ObjectStack Protocol Architects +**Builds on**: [ADR-0003](./0003-package-as-first-class-citizen.md) (package as first-class citizen), [ADR-0005](./0005-metadata-customization-overlay.md) (artifact vs runtime overlay precedence), [ADR-0008](./0008-metadata-repository-and-change-log.md) (metadata repository, `MetaRef` identity), [ADR-0010](./0010-metadata-protection-model.md) (package provenance / `_packageId` stamping) +**Consumers**: `@objectstack/objectql` (`SchemaRegistry.registerItem`, `ObjectQL.registerApp`), package authors, CLI/CI install path +**Surfaced by**: ADR-0046 review (doc naming) — generalised here into its own work item. + +--- + +## TL;DR + +The metadata registry key is `org/type/name` — it has **no package +coordinate** (`refKey` in `packages/metadata-core/src/types.ts`). Object +names dodge collisions because the kernel namespace-prefix-validates them +(they map to physical table names). But **bare-named UI/automation metadata +is not prefix-validated**: `page`, `dashboard`, `flow`, `app`, `action`, +`doc` only require snake_case. So two installed packages that each define a +`page` named `home` produce the same logical key, and the second +registration **silently shadows the first** — worse than the object case, +which fails loudly at the DB. + +**Decision:** detect cross-package same-key collisions in the code-defined +**base layer** at registration time and raise an explicit, actionable error +naming both packages and the type/name. Do **not** retrofit +namespace-prefix enforcement onto every existing bare-named type (large +migration cost). Prefix stays a *recommended convention* — and brand-new +types can be strict from day one. + +## 1. Context + +### 1.1 The registry key carries no package coordinate + +Metadata identity is `(org, type, name)`: + +```ts +// packages/metadata-core/src/types.ts +export function refKey(ref: Pick): string { + return `${ref.org}/${ref.type}/${ref.name}`; +} +``` + +Nothing in that key says *which package* a `system/page/home` came from. +For objects this is harmless: object names are validated against a +namespace prefix in the kernel (`validateNamespacePrefix` in +`packages/spec/src/stack.zod.ts`) because they become physical table names, +so two packages cannot both ship `account` — and if they tried, the second +`CREATE TABLE` fails **loudly** at the database. + +Bare-named UI/automation metadata has no such backstop. `page`, +`dashboard`, `flow`, `app`, `action`, and (as of ADR-0046) `doc` only +require `SnakeCaseIdentifierSchema`. Two packages can each legitimately +declare a `page` named `home`. + +### 1.2 How the silent shadowing actually happens + +In the objectql `SchemaRegistry`, generic (non-object) metadata lives in a +two-level map and is stored under a **composite** key when a package id is +present: + +```ts +// packages/objectql/src/registry.ts — registerItem() +const storageKey = packageId ? `${packageId}:${baseName}` : baseName; +collection.set(storageKey, item); +``` + +So `crm` and `hr` both shipping `page/home` do **not** overwrite the same +map entry — they sit under `crm:home` and `hr:home`. The shadowing surfaces +one layer up, at **read** time: + +```ts +// getItem() — returns the FIRST composite key matching `:` +for (const [key, item] of collection) { + if (key.endsWith(`:${name}`)) return item as T; +} +``` + +`getItem('page', 'home')` returns whichever entry the `Map` iterates first +— i.e. **whichever package was registered first**. The other package's +`home` is unreachable by name, with no error and no warning. It is +last-write-wins (here, *first-registered-wins*) and entirely silent — the +exact failure ADR-0046's review flagged for `doc`, generalised to every +bare-named type. + +### 1.3 What is *not* a collision (and must keep working) + +The same `(type, name)` is written more than once for entirely legitimate +reasons. The guard must not break these: + +- **Same-package reload.** Re-registering a package (dev reload, idempotent + install) re-writes `crm:home` with `crm`'s own value. Same owner — not a + collision. +- **Runtime / DB overlay (ADR-0005).** A runtime-authored row in + `sys_metadata` overlays a packaged artifact. It is registered under the + **bare** key with no real package provenance (or carries the + `'sys_metadata'` rehydration sentinel as `_packageId`). This is the + sanctioned override path; `registerItem` already emits an artifact-vs-DB + *shadowing warning* for it and must continue to allow it. +- **Object ownership / extension.** Objects use a separate + contributor model (`own` / `extend`, `registerObject`) and never flow + through this guard. +- **Navigation contributions (ADR-0029).** A package injecting nav items + into an app it does not own uses `appNavContributions`, not a duplicate + `app` registration. + +The bug is specifically a **base-layer collision between two different code +packages**. Provenance is already available to tell them apart: +ADR-0010 stamps every artifact-registered item with `_packageId` +(`applyProtection`), and the registration call passes the owning package id +explicitly. + +## 2. Goals & non-goals + +**Goals** +- Make a cross-package base-layer collision a loud, actionable failure at + registration/install time, naming both packages and the type/name. +- Cost-cheap: piggyback on the registration path, which already reads the + collection by key. +- Zero false positives on overlays, same-package reloads, objects, and nav + contributions. + +**Non-goals** +- Retrofitting namespace-prefix enforcement onto existing bare-named types + (`page`, `flow`, …). That is a breaking rename for every shipped package + and is out of scope. +- Changing the `org/type/name` key shape or adding a package column to + `sys_metadata`. +- Cross-**org** overlay semantics (unchanged; ADR-0005 governs them). + +## 3. Decision + +### 3.1 Detect, error, name the culprits + +At registration time, when a code package registers a bare-named generic +item, refuse it if a **different** code package already owns the same +`(type, name)` in the base layer. The error names both packages, the type, +and the name, and points at the fix. + +Detection lives at the single choke point that every installed package's +metadata arrays pass through — `SchemaRegistry.registerItem` +(`packages/objectql/src/registry.ts`). `ObjectQL.registerApp` (and the +nested-plugin loop) delegate to it, so guarding it once covers manifest +metadata and plugin metadata alike. The check: + +> `registerItem` is called with a real `packageId`, **and** an existing +> entry for the same `(type, name)` carries a *different* real `_packageId` +> (truthy, and not the `'sys_metadata'` sentinel) → `MetadataCollisionError`. + +Same-package writes (`owner === incoming`), bare/overlay rows (no real +owner), and the `'sys_metadata'` sentinel are all excluded, so the +legitimate cases in §1.3 pass through untouched. Detection scans the live +collection — exactly as `getItem`/`unregisterItem` already do — so there is +no parallel index to drift across `reset`/`unregister`. + +### 3.2 Policy is `error` by default, `warn` as an escape hatch + +`collisionPolicy` defaults to `'error'`. A `'warn'` mode (constructor option +or `OS_METADATA_COLLISION=warn`) downgrades to a logged warning and lets the +registration proceed, for deliberate, temporary migrations (e.g. renaming a +colliding page across two packages in flight). The default is loud; the +opt-out is explicit and discoverable from the error message itself. + +### 3.3 Why detection over prefix enforcement + +Two ways to kill the collision: + +1. **Prefix enforcement** — require every bare-named type's `name` to start + with the package namespace, like objects. Closes the hole at the source, + but renames the entire installed base (every `page`/`flow`/`action` in + every shipped package and pilot), breaks cross-references, and forces a + coordinated migration. High cost, high blast radius. +2. **Collision detection** (this ADR) — leave existing names alone; make the + *clash* an error. Near-zero migration cost, the registration path already + reads the key, and the failure is actionable. + +We choose (2). Prefixing remains the **recommended convention** — the error +message literally suggests `_` — and may be surfaced as a +non-fatal *lint warning* in the CLI later, but it is **not retroactively +enforced** on legacy types. + +### 3.4 New types can be strict from day one + +A type introduced *after* this ADR has no installed base to migrate, so it +can adopt namespace-prefix validation immediately at the spec layer and get +both guarantees (no collision *and* self-describing names). ADR-0046's +`doc` is the first candidate: its CLI already enforces namespace-prefixed +snake_case names at build time, so `doc` is effectively prefix-strict at the +authoring boundary while this ADR's registry guard is its runtime backstop. +The general rule: **legacy types → detect; new types → prefix-strict + +detect.** + +## 4. Consequences + +- A genuine cross-package clash now fails fast at boot/install with a + message identifying both packages — instead of a coin-flip over which + package's `home` page a user sees. The hazard moves from *silent at read* + to *loud at registration*. +- Package authors who unknowingly relied on first-registered-wins will get + an error; the fix (rename with a namespace prefix, or `warn` during + migration) is in the message. +- No change to the key shape, the overlay model, or object/nav paths. +- Follow-ups (not in this ADR): a CLI lint that flags non-prefixed + bare-named metadata as a warning; prefix-strict spec validation for the + next net-new bare-named type. + +## 5. Implementation notes + +- `packages/objectql/src/registry.ts`: `MetadataCollisionError` (exported), + `isRealPackage` helper (excludes the `'sys_metadata'` sentinel), + `collisionPolicy` option + `OS_METADATA_COLLISION` env, the guard in + `registerItem`, and `findOtherPackageOwner` (live-collection scan). +- Tests: `registry-cross-package-collision.test.ts` (unit — error/warn, + same-package reload, overlay, sentinel, distinct names) and + `engine-cross-package-collision.test.ts` (end-to-end through + `ObjectQL.registerApp`). +- The `metadata-core` repository (`refKey`, `put`) is the *conceptual* root + of the missing package coordinate, but its optimistic-concurrency + `parentVersion` check already rejects a blind base-layer double-create + with `ConflictError`; the genuinely *silent* path is the objectql + `SchemaRegistry` read resolution, which is where enforcement lands. diff --git a/packages/objectql/src/engine-cross-package-collision.test.ts b/packages/objectql/src/engine-cross-package-collision.test.ts new file mode 100644 index 0000000000..bddff89497 --- /dev/null +++ b/packages/objectql/src/engine-cross-package-collision.test.ts @@ -0,0 +1,57 @@ +// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * ADR-0048 — end-to-end: the cross-package collision guard fires through the + * real `ObjectQL.registerApp` entry point (not just the registry unit), since + * that is the choke point every installed package's metadata arrays flow + * through. Uses a real engine + real registry (no mock) on purpose. + */ + +import { describe, it, expect } from 'vitest'; +import { ObjectQL } from './engine'; +import { MetadataCollisionError } from './registry'; + +describe('ObjectQL.registerApp — cross-package collision (ADR-0048)', () => { + it('throws when a second package registers a bare-named page already owned by another', () => { + const engine = new ObjectQL(); + engine.registerApp({ + id: 'com.acme.crm', + pages: [{ name: 'home', title: 'CRM Home' }], + }); + + expect(() => + engine.registerApp({ + id: 'com.acme.hr', + pages: [{ name: 'home', title: 'HR Home' }], + }), + ).toThrowError(MetadataCollisionError); + }); + + it('allows two packages to define same-named pages once namespaced apart', () => { + const engine = new ObjectQL(); + expect(() => { + engine.registerApp({ + id: 'com.acme.crm', + pages: [{ name: 'crm_home', title: 'CRM Home' }], + }); + engine.registerApp({ + id: 'com.acme.hr', + pages: [{ name: 'hr_home', title: 'HR Home' }], + }); + }).not.toThrow(); + }); + + it('allows the same package to be re-registered (idempotent reload)', () => { + const engine = new ObjectQL(); + engine.registerApp({ + id: 'com.acme.crm', + pages: [{ name: 'home', title: 'v1' }], + }); + expect(() => + engine.registerApp({ + id: 'com.acme.crm', + pages: [{ name: 'home', title: 'v2' }], + }), + ).not.toThrow(); + }); +}); diff --git a/packages/objectql/src/registry-cross-package-collision.test.ts b/packages/objectql/src/registry-cross-package-collision.test.ts new file mode 100644 index 0000000000..1589f0e360 --- /dev/null +++ b/packages/objectql/src/registry-cross-package-collision.test.ts @@ -0,0 +1,126 @@ +// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * ADR-0048 — cross-package metadata collision detection. + * + * Bare-named generic metadata (`page`, `dashboard`, `flow`, `action`, `doc`, + * …) carries no package coordinate in the registry key, so two installed + * packages defining the same `(type, name)` would silently shadow each other + * at read time (last-write-wins). These tests pin the guard: real + * cross-package base-layer collisions fail loudly, while same-package reloads + * and legitimate runtime/DB overlays pass through untouched. + */ + +import { describe, it, expect, beforeEach, vi, afterEach } from 'vitest'; +import { SchemaRegistry, MetadataCollisionError } from './registry'; + +describe('SchemaRegistry — cross-package collision (ADR-0048)', () => { + let registry: SchemaRegistry; + + beforeEach(() => { + registry = new SchemaRegistry({ multiTenant: false, collisionPolicy: 'error' }); + registry.logLevel = 'silent'; + }); + + it('throws when two different packages register the same (type, name)', () => { + registry.registerItem('page', { name: 'home', title: 'CRM Home' }, 'name', 'crm'); + expect(() => + registry.registerItem('page', { name: 'home', title: 'HR Home' }, 'name', 'hr'), + ).toThrowError(MetadataCollisionError); + }); + + it('error names both packages and the type/name', () => { + registry.registerItem('flow', { name: 'on_create' }, 'name', 'crm'); + try { + registry.registerItem('flow', { name: 'on_create' }, 'name', 'hr'); + throw new Error('expected a collision error'); + } catch (e) { + expect(e).toBeInstanceOf(MetadataCollisionError); + const err = e as MetadataCollisionError; + expect(err.type).toBe('flow'); + expect(err.name_).toBe('on_create'); + expect(err.existingPackageId).toBe('crm'); + expect(err.incomingPackageId).toBe('hr'); + expect(err.message).toContain('crm'); + expect(err.message).toContain('hr'); + expect(err.message).toContain('flow/on_create'); + } + }); + + it('does NOT throw when the same package re-registers the same name (idempotent reload)', () => { + registry.registerItem('page', { name: 'home', title: 'v1' }, 'name', 'crm'); + expect(() => + registry.registerItem('page', { name: 'home', title: 'v2' }, 'name', 'crm'), + ).not.toThrow(); + // The latest value from the same package wins (overwrite under the same key). + expect(registry.getItem('page', 'home')?.title).toBe('v2'); + }); + + it('does NOT throw for a runtime/DB overlay over a packaged item (ADR-0005 overlay)', () => { + // Package ships the artifact under a composite key… + registry.registerItem('page', { name: 'home', title: 'packaged' }, 'name', 'crm'); + // …and a runtime-authored row (no packageId) overlays it under the bare key. + expect(() => + registry.registerItem('page', { name: 'home', title: 'runtime' }, 'name'), + ).not.toThrow(); + }); + + it('does NOT throw when a package ships over a pre-existing bare/runtime row', () => { + // Runtime/DB row registered first (no packageId)… + registry.registerItem('page', { name: 'home', title: 'runtime' }, 'name'); + // …then a package ships the same name. This is the artifact-vs-DB case, + // handled by the existing shadowing warning, not a cross-package error. + expect(() => + registry.registerItem('page', { name: 'home', title: 'packaged' }, 'name', 'crm'), + ).not.toThrow(); + }); + + it('treats the sys_metadata rehydration sentinel as a non-owner (no collision)', () => { + // An item rehydrated from sys_metadata carries _packageId='sys_metadata'. + registry.registerItem('page', { name: 'home', _packageId: 'sys_metadata' }, 'name'); + expect(() => + registry.registerItem('page', { name: 'home', title: 'packaged' }, 'name', 'crm'), + ).not.toThrow(); + }); + + it('does NOT throw for the same name owned by the same package across types', () => { + registry.registerItem('page', { name: 'home' }, 'name', 'crm'); + expect(() => + registry.registerItem('dashboard', { name: 'home' }, 'name', 'hr'), + ).not.toThrow(); + }); + + it('does NOT throw for different names across packages', () => { + registry.registerItem('page', { name: 'crm_home' }, 'name', 'crm'); + expect(() => + registry.registerItem('page', { name: 'hr_home' }, 'name', 'hr'), + ).not.toThrow(); + }); + + describe("collisionPolicy: 'warn'", () => { + let warnSpy: ReturnType; + + beforeEach(() => { + registry = new SchemaRegistry({ multiTenant: false, collisionPolicy: 'warn' }); + registry.logLevel = 'silent'; + warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}); + }); + + afterEach(() => { + warnSpy.mockRestore(); + }); + + it('warns instead of throwing, and registers both items', () => { + registry.registerItem('page', { name: 'home', title: 'CRM' }, 'name', 'crm'); + expect(() => + registry.registerItem('page', { name: 'home', title: 'HR' }, 'name', 'hr'), + ).not.toThrow(); + expect(warnSpy).toHaveBeenCalled(); + const msg = warnSpy.mock.calls.map((c) => String(c[0])).join('\n'); + expect(msg).toContain('Cross-package metadata collision'); + // Both survive under distinct composite keys; the artifact lookup still + // resolves an item (read-time shadowing is what the error guards against). + expect(registry.getItem('page', 'home')).toBeDefined(); + }); + }); +}); diff --git a/packages/objectql/src/registry.ts b/packages/objectql/src/registry.ts index b2278072e3..928b0805ac 100644 --- a/packages/objectql/src/registry.ts +++ b/packages/objectql/src/registry.ts @@ -141,6 +141,23 @@ export interface SchemaRegistryOptions { * (useful in tests). */ multiTenant?: boolean; + + /** + * Policy for cross-package base-layer metadata collisions (ADR-0048) — two + * different code packages registering a bare-named generic item under the + * same `(type, name)`. + * + * - `'error'` (default): throw {@link MetadataCollisionError} at registration + * time, naming both packages and the type/name. Makes the otherwise-silent + * last-write-wins shadowing a loud, actionable failure. + * - `'warn'`: log a warning and let the registration proceed. For deliberate + * migrations where a collision is temporarily expected. + * + * Sourced from `OS_METADATA_COLLISION` (`warn` to downgrade) when not set + * explicitly. Legitimate runtime/DB overlays and same-package reloads are + * never treated as collisions regardless of this setting. + */ + collisionPolicy?: 'error' | 'warn'; } /** @@ -283,6 +300,63 @@ export function applySystemFields( }; } +/** + * The rehydration sentinel stamped on items loaded back from `sys_metadata` + * (runtime/DB overlay rows). It is NOT a real owning code package, so it must + * never participate in cross-package collision detection (ADR-0048). + */ +const SYS_METADATA_OWNER = 'sys_metadata'; + +/** + * True when `pkg` identifies a genuine code package (an artifact owner), as + * opposed to absent provenance or the `sys_metadata` runtime-overlay sentinel. + * Cross-package collision detection (ADR-0048) only compares real owners so + * that legitimate runtime/DB overlays never look like a base-layer collision. + */ +function isRealPackage(pkg: unknown): pkg is string { + return typeof pkg === 'string' && pkg.length > 0 && pkg !== SYS_METADATA_OWNER; +} + +/** + * Raised when two **different** code packages register a generic (non-object) + * metadata item under the same `(type, name)` in the code-defined base layer + * (ADR-0048). + * + * The registry key for bare-named UI/automation metadata (`page`, `dashboard`, + * `flow`, `app`, `action`, `doc`, …) carries no package coordinate — those + * names are only snake_case-validated, never namespace-prefix-validated the way + * object names are. So two installed packages that each define e.g. a `page` + * named `home` collide on the same logical key, and bare-name read resolution + * (`getItem`) would silently return whichever the registry iterates first, + * leaving the other package's item unreachable. This error makes that hazard + * loud at registration/install time instead of silent at read time. + */ +export class MetadataCollisionError extends Error { + readonly type: string; + readonly name_: string; + readonly existingPackageId: string; + readonly incomingPackageId: string; + + constructor(type: string, name: string, existingPackageId: string, incomingPackageId: string) { + super( + `Cross-package metadata collision: ${type}/${name} is registered by ` + + `package "${existingPackageId}" and package "${incomingPackageId}". ` + + `Bare-named ${type} metadata has no package coordinate in the registry, ` + + `so the second registration would silently shadow the first ` + + `(last-write-wins at read time). Rename one of them (a namespace prefix ` + + `such as "_${name}" is recommended), or, if this is a ` + + `deliberate migration, set OS_METADATA_COLLISION=warn to downgrade to a ` + + `warning. See ADR-0048.`, + ); + this.name = 'MetadataCollisionError'; + this.type = type; + // `name` is the Error message-class name; store the metadata name separately. + this.name_ = name; + this.existingPackageId = existingPackageId; + this.incomingPackageId = incomingPackageId; + } +} + export class SchemaRegistry { // ========================================== // Logging control @@ -294,6 +368,9 @@ export class SchemaRegistry { /** Whether to auto-inject multi-tenant system fields. */ private readonly multiTenant: boolean; + /** Cross-package base-layer collision policy (ADR-0048). */ + private readonly collisionPolicy: 'error' | 'warn'; + constructor(options: SchemaRegistryOptions = {}) { if (options.multiTenant !== undefined) { this.multiTenant = options.multiTenant; @@ -302,6 +379,12 @@ export class SchemaRegistry { this.multiTenant = String(readEnvWithDeprecation('OS_MULTI_ORG_ENABLED', 'OS_MULTI_TENANT') ?? 'false').toLowerCase() !== 'false'; } + + // ADR-0048 — default to a loud error on cross-package collision; allow an + // env opt-out for deliberate migrations. + this.collisionPolicy = + options.collisionPolicy ?? + ((process.env.OS_METADATA_COLLISION ?? '').toLowerCase() === 'warn' ? 'warn' : 'error'); } get logLevel(): RegistryLogLevel { return this._logLevel; } @@ -738,6 +821,32 @@ export class SchemaRegistry { this.log(`[Registry] Overwriting ${type}: ${storageKey}`); } + // ADR-0048 — cross-package base-layer collision. When a code package + // registers a bare-named generic item, refuse it loudly if a DIFFERENT + // code package already owns the same (type, name). Without this guard the + // two items live under distinct composite keys but bare-name resolution + // (`getItem`) returns whichever the Map iterates first, silently shadowing + // the loser — last-write-wins with no diagnostic. + // + // What is deliberately NOT a collision (these must pass through): + // - Same package re-registering the same name (idempotent reload): + // `conflictOwner` excludes `packageId` itself. + // - Runtime/DB overlay rows registered under the bare key with no real + // package provenance (or the `sys_metadata` sentinel): that is the + // legitimate ADR-0005 overlay path, already surfaced by the + // artifact-vs-DB warning below. + if (isRealPackage(packageId)) { + const conflictOwner = this.findOtherPackageOwner(collection, baseName, packageId); + if (conflictOwner) { + const err = new MetadataCollisionError(type, baseName, conflictOwner, packageId); + if (this.collisionPolicy === 'warn') { + console.warn(`[Registry] ${err.message}`); + } else { + throw err; + } + } + } + // Artifact-vs-DB collision warning. When a code package ships an item // whose name already exists as a DB-only entry (registered earlier // without a packageId — typically rehydrated from sys_metadata by @@ -763,6 +872,28 @@ export class SchemaRegistry { this.log(`[Registry] Registered ${type}: ${storageKey}`); } + /** + * Find a code package OTHER than `incoming` that already owns `baseName` in + * `collection` (ADR-0048 cross-package collision detection). Scans the live + * collection — like {@link getItem} / {@link unregisterItem} — so it always + * reflects current state with no parallel index to drift across + * reset/unregister. Returns the conflicting owner's package id, or undefined + * when the name is free or only held by the same package / a runtime overlay. + */ + private findOtherPackageOwner( + collection: Map, + baseName: string, + incoming: string, + ): string | undefined { + for (const [key, item] of collection) { + // Same logical name only — the bare key or any `:` key. + if (key !== baseName && !key.endsWith(`:${baseName}`)) continue; + const owner = item?._packageId; + if (isRealPackage(owner) && owner !== incoming) return owner; + } + return undefined; + } + /** * Validate Metadata against Spec Zod Schemas */