diff --git a/docs/adr/0048-cross-package-metadata-collision.md b/docs/adr/0048-cross-package-metadata-collision.md index fcd9c05698..9d16094e06 100644 --- a/docs/adr/0048-cross-package-metadata-collision.md +++ b/docs/adr/0048-cross-package-metadata-collision.md @@ -1,4 +1,4 @@ -# ADR-0048: Cross-package metadata collision — package-scoped identity, namespace install gate, container-scoped resolution +# ADR-0048: Cross-package metadata collision — package-id identity, namespace install gate, package-scoped resolution **Status**: Revised (2026-06-13) — supersedes the original *per-item collision detection* framing. The runtime guard shipped under the original proposal is @@ -42,15 +42,22 @@ resolution**, not write-time clash detection: *different* installed package is **refused** — making explicit and early a constraint the object/table layer already enforces implicitly (a duplicate `CREATE TABLE crm_account` fails loudly at the DB). -3. **Resolution is container-scoped.** UI/automation metadata resolves within - its package/app container, so a cross-package clash on a bare name is - *structurally impossible* rather than *detected after the fact*. -4. **Per-item runtime detection (shipped) is demoted to a same-package - authoring backstop** + an authoring-time lint — it now only catches an - author shipping two `page/home` *within one package*. +3. **Resolution is package-scoped, keyed on the package id.** A bare name + resolves within the caller's package first (`getItem(type, name, + currentPackageId?)`; items already carry `_packageId`). Because package ids + are globally unique, two packages shipping `page/home` coexist and each + caller resolves to its own — a cross-package clash *cannot mis-resolve* for + any caller carrying its package id. +4. **The per-item cross-package throw retires.** Distinct packages are always + disambiguable by package id, so what the original guard flagged as a + collision is now the *supported* coexistence case. Same-package writes + overwrite (idempotent reload); the `os lint` namespace-prefix rule keeps + authoring hygiene. 5. **Namespace rename-on-install is an explicit non-goal for now** (deep rewrite of every object name, cross-reference, and formula). v1 is *refuse-on-conflict*; rename is a separate future work item. +6. **The package-id URL is transparent; a per-tenant namespace alias is an + optional sugar** (`/apps/crm` → `com.acme.crm`), never the stored identity. ## 1. Context @@ -203,35 +210,52 @@ DB). The gate just makes that constraint **explicit and early** (a clean pre-install check) instead of a half-applied install blowing up at `CREATE TABLE`. -Reserved namespaces (`base`, `system`, `sys`) are exempt, as today. +Note the gate is **not load-bearing for routing** under §3.1 — routing keys on +the globally-unique package id, which is correct with or without the gate +(local dev, build-time, federation). The gate serves the object/table layer +and is the basis for the optional per-tenant alias (§3.6). Reserved namespaces +(`base`, `system`, `sys`) are exempt, as today. -### 3.3 Resolution is container-scoped (prefer-local, qualify-to-cross) +### 3.3 Resolution is package-scoped (prefer-local, qualify-to-cross) `getItem`/route resolution resolves a bare name **within the current package -container first**: +first**, keyed on the **package id**: - Within `/apps//…`, a bare `page/home` resolves to *this - package's* `home`. The current package is already known from the route and - from React context (`activeApp`), so this is a single-field scoping, not a - signature change at every call site. + package's* `home`. The current package id is already known from the route and + from React context (`activeApp._packageId`), so this is a single-field + scoping — `getItem(type, name, currentPackageId?)` — not a signature change + at every call site. Items already carry their owner (`_packageId`, ADR-0010), + so the match is `_packageId === currentPackageId`. - A **deliberate cross-package reference** uses a qualified form - (`:` / `:`) — the only place a second - package's metadata is reachable, and it is explicit. - -Because two packages can never share a namespace (§3.2) and bare names resolve -inside the container (§3.3), **a cross-package clash on `page/home` cannot -occur** — there is nothing left to detect at the cross-package level. - -### 3.4 Per-item runtime detection is demoted to a same-package backstop - -The shipped guard (`MetadataCollisionError`, `collisionPolicy`, -`OS_METADATA_COLLISION`, `findOtherPackageOwner` in -`packages/objectql/src/registry.ts`) **stays**, but its role narrows: with the -namespace gate (§3.2) eliminating cross-package clashes, the only remaining -duplicate is **one package shipping two items with the same `(type, name)`** — -an authoring mistake, best caught by the `naming/namespace-prefix` lint in -`os lint` at authoring time, with the runtime guard as the last-line backstop. -The guard is cheap and already there; no reason to remove it. + (`:`) — the only place a second package's metadata is + reachable, and it is explicit. + +The disambiguation rests on the **package id being globally unique**, *not* on +the namespace gate: two packages legitimately ship `page/home`, store under +distinct composite keys (`com.acme.crm:home`, `com.acme.hr:home`), and each +caller resolves to its own package's item. **A cross-package clash on +`page/home` therefore cannot mis-resolve for any caller that carries its +package id** — which every routed UI surface does. A *context-free* read +(`getItem` with no package id) is best-effort: it returns the first match and +the caller is expected to pass the package id when it cares. + +### 3.4 Per-item cross-package detection retires; same-package overwrite stays + +The original proposal's per-item guard threw `MetadataCollisionError` whenever +two **different** packages registered the same `(type, name)`. Under +package-scoped resolution that is exactly the case we now *support*: package +ids are always distinct, so prefer-local always disambiguates two different +packages — there is no unresolvable cross-package clash to detect. The +cross-package **throw is retired**; two distinct packages coexist on the same +bare name by construction. + +What remains is the narrow, genuinely-ambiguous case the guard still earns its +keep on: **a write with no real package provenance** (a `sys_metadata`/runtime +overlay) is governed by the ADR-0005 overlay precedence (artifact-vs-DB warning, +unchanged), and **same-package re-registration** simply overwrites (idempotent +reload). Authoring-time hygiene — an author shipping two `page/home` in one +package — stays covered by the `naming/namespace-prefix` lint in `os lint`. ### 3.5 Namespace rename-on-install is deferred (non-goal) @@ -241,16 +265,34 @@ formula / view / flow that names `crm_account`. That is a deep rewrite, not a URL change. v1 is **refuse-on-conflict** (§3.2). Rename-on-install is recorded as future work, not part of this decision. +### 3.6 The package-id URL is transparent; a friendly alias is optional + +A reverse-domain URL — `/apps/com.acme.crm/page/home` — is **self-describing**: +vendor (`acme`), product (`crm`), and surface are legible at a glance, the way +Android package names, Java FQNs, and `k8s` `namespace/name` are. For a host +that runs third-party packages this is a feature, not noise: "which package is +this page from?" is answerable from the URL alone — a trust, support, and +debugging win — and one vendor's `crm` cannot be mistaken for another's. + +Its one real cost is **length**. When a short URL is wanted, a host MAY expose a +**per-tenant friendly alias** — `/apps/crm` resolving to `com.acme.crm` — +because the namespace gate (§3.2) makes the namespace unique *within a tenant*. +The alias is a tenant-local presentation convenience layered over the canonical +package-id route; it is **never** the stored identity. Canonical = package id +(robust, coordination-free); alias = namespace (pretty, tenant-scoped). The +alias is optional and out of scope for the phases below. + ## 4. Consequences - **Two vendors' packages coexist.** `com.acme.crm` and `com.beta.crm` install side by side; each `home` page is reachable under its own `/apps//…` container. The marketplace becomes viable for common-named packages. -- **One per-package gate replaces N per-item checks.** Cross-package safety is - an `O(1)`-per-package namespace check at install, not an `O(every - page/dashboard/flow)` registration scan — cheaper, right granularity, and it - fires *before* a half-install. +- **Cross-package safety becomes structural, not detected.** Package-scoped + resolution (§3.3) keyed on the unique package id means two packages never + mis-resolve a shared bare name, so the `O(every page/dashboard/flow)` + per-item registration scan is retired. The remaining install-time work is a + single `O(1)`-per-package namespace check for the object/table layer (§3.2). - **Object and UI metadata share one scope model.** The package container scopes both; the long-standing "objects are safe, UI metadata isn't" asymmetry disappears, with **zero artifact renames**. @@ -264,27 +306,33 @@ as future work, not part of this decision. ## 5. Implementation phasing -Status legend: **[done]** shipped under the original proposal · **[proposed]** -this revision. +Status legend: **[done]** shipped · **[proposed]** not yet built · +**[deferred]** out of scope here. -- **[done]** Runtime same-package backstop: `MetadataCollisionError`, - `isRealPackage`, `collisionPolicy` + `OS_METADATA_COLLISION`, the guard in - `SchemaRegistry.registerItem`, `findOtherPackageOwner` - (`packages/objectql/src/registry.ts`); tests - `registry-cross-package-collision.test.ts` and - `engine-cross-package-collision.test.ts`. - **[done]** Authoring lint: `naming/namespace-prefix` in `os lint` (warns on non-prefixed `app`/`page`/`dashboard`/`flow`/`action`/`report`/`dataset`; exempts the namespace-named app per ADR-0019 and `sys_` names; warning-only). -- **[proposed] Phase 1 — install-time namespace gate.** In the package install - path, refuse a package whose `manifest.namespace` is already owned by a - different installed package; actionable error naming both packages. Backed by - the installed-package registry (`InstalledPackage`). -- **[proposed] Phase 2 — container-scoped resolution.** Thread the current - package/app into `SchemaRegistry.getItem` (prefer-local) and into `objectui` - metadata resolution (`MetadataProvider` / the `pages.find(name===)` sites), - collapsing the frontend's first-match-wins mirror of the bug. Routes keyed on - `packageId`; `app.label` carries display. +- **[done] Phase 1 — install-time namespace gate.** `NamespaceConflictError` + + the gate in `SchemaRegistry.installPackage` (refuses a package whose + `manifest.namespace` is already owned by a different installed package; + same-package reload and shareable `base`/`system`/`sys` exempt; + `OS_METADATA_COLLISION=warn` downgrades). Tests: + `registry-namespace-install-gate.test.ts`. +- **[done] Phase 2 (backend) — package-scoped resolution.** + `getItem(type, name, currentPackageId?)` prefers the current package's + composite entry, keeping ADR-0005 overlay precedence; backward compatible. + The per-item **cross-package throw is retired** (§3.4) — two distinct + packages coexist on the same bare name. Tests: + `registry-prefer-local-resolution.test.ts`; the original + `*-cross-package-collision.test.ts` are rewritten from "throws" to + "coexists + prefer-local resolves". +- **[done] Phase 2 (frontend) — prefer-local in objectui.** + `preferLocal(list, name, ownerPackageId)` keyed on `_packageId`, wired at the + page/dashboard/report/header bare-name sites. +- **[proposed] Phase 2 (frontend, remaining) — package-id routing.** Move the + `/apps/:appName` segment to the package id and select the active app by + `_packageId` (closing the app-selection ambiguity that `appName` leaves + open). Optional: the per-tenant namespace alias (§3.6). - **[proposed] Phase 3 — qualified cross-package references.** Define and document the `:` reference form for the deliberate cross-package case (nav contributions, shared pages); resolution falls back @@ -298,10 +346,10 @@ this revision. the missing package coordinate; its optimistic-concurrency `parentVersion` check already rejects a blind base-layer double-create with `ConflictError`. The genuinely *silent* path was the objectql `SchemaRegistry` read - resolution — which §3.3 makes container-scoped. + resolution — which §3.3 makes package-scoped. - `objectui` route inventory (for Phase 2): metadata reachable by name divides into (a) already-safe — `object`/`view` (kernel namespace), `component` (`:ns/:name` segment), `doc` (ADR-0046 authoring prefix), marketplace/package - routes (already keyed on `packageId`); (b) container-scoped via this ADR — + routes (already keyed on `packageId`); (b) package-scoped via this ADR — `page`/`dashboard`/`report`; (c) one-off — `app` (now keyed on `packageId`); (d) intentionally global — `action`, Studio's `metadata/:type/:name` admin. diff --git a/packages/objectql/src/engine-cross-package-collision.test.ts b/packages/objectql/src/engine-cross-package-collision.test.ts index bddff89497..2cfed0a09a 100644 --- a/packages/objectql/src/engine-cross-package-collision.test.ts +++ b/packages/objectql/src/engine-cross-package-collision.test.ts @@ -1,18 +1,18 @@ // 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. + * ADR-0048 §3.4 — end-to-end: two packages shipping the same bare-named page + * coexist through the real `ObjectQL.registerApp` entry point (the choke point + * every installed package's metadata arrays flow through). Uses a real engine + + * real registry (no mock) on purpose. Package-scoped resolution keeps each + * package's `home` reachable; there is no cross-package throw. */ 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', () => { +describe('ObjectQL.registerApp — cross-package coexistence (ADR-0048 §3.4)', () => { + it('lets a second package register a bare-named page already owned by another', () => { const engine = new ObjectQL(); engine.registerApp({ id: 'com.acme.crm', @@ -24,34 +24,31 @@ describe('ObjectQL.registerApp — cross-package collision (ADR-0048)', () => { id: 'com.acme.hr', pages: [{ name: 'home', title: 'HR Home' }], }), - ).toThrowError(MetadataCollisionError); + ).not.toThrow(); }); - it('allows two packages to define same-named pages once namespaced apart', () => { + it('resolves each package\'s page by package id', () => { + const engine = new ObjectQL(); + engine.registerApp({ id: 'com.acme.crm', pages: [{ name: 'home', title: 'CRM Home' }] }); + engine.registerApp({ id: 'com.acme.hr', pages: [{ name: 'home', title: 'HR Home' }] }); + + expect(engine.registry.getItem('page', 'home', 'com.acme.crm')?.title).toBe('CRM Home'); + expect(engine.registry.getItem('page', 'home', 'com.acme.hr')?.title).toBe('HR Home'); + }); + + it('allows two packages with differently-named pages (trivially)', () => { 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' }], - }); + 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' }], - }); + engine.registerApp({ id: 'com.acme.crm', pages: [{ name: 'home', title: 'v1' }] }); expect(() => - engine.registerApp({ - id: 'com.acme.crm', - pages: [{ name: 'home', title: 'v2' }], - }), + 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 index 1589f0e360..ad76882c90 100644 --- a/packages/objectql/src/registry-cross-package-collision.test.ts +++ b/packages/objectql/src/registry-cross-package-collision.test.ts @@ -1,126 +1,81 @@ // Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. /** - * ADR-0048 — cross-package metadata collision detection. + * ADR-0048 §3.4 — cross-package coexistence. * * 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. + * …) carries no package coordinate in the *logical* key, but the registry + * stores each owner's item under a distinct composite key (`:`). + * Because package ids are globally unique, package-scoped resolution + * (`getItem(type, name, currentPackageId)`) routes each caller to its own + * package's item — so two installed packages shipping the same bare name + * legitimately COEXIST. The original per-item cross-package throw is retired; + * these tests pin that coexistence and the unchanged not-a-collision cases + * (runtime/DB overlay, sys_metadata sentinel, same-package reload). */ -import { describe, it, expect, beforeEach, vi, afterEach } from 'vitest'; -import { SchemaRegistry, MetadataCollisionError } from './registry'; +import { describe, it, expect, beforeEach } from 'vitest'; +import { SchemaRegistry } from './registry'; -describe('SchemaRegistry — cross-package collision (ADR-0048)', () => { +describe('SchemaRegistry — cross-package coexistence (ADR-0048 §3.4)', () => { let registry: SchemaRegistry; beforeEach(() => { - registry = new SchemaRegistry({ multiTenant: false, collisionPolicy: 'error' }); + registry = new SchemaRegistry({ multiTenant: false }); registry.logLevel = 'silent'; }); - it('throws when two different packages register the same (type, name)', () => { - registry.registerItem('page', { name: 'home', title: 'CRM Home' }, 'name', 'crm'); + it('lets two different packages register the same (type, name) without error', () => { + registry.registerItem('page', { name: 'home', title: 'CRM Home' }, 'name', 'com.acme.crm'); expect(() => - registry.registerItem('page', { name: 'home', title: 'HR Home' }, 'name', 'hr'), - ).toThrowError(MetadataCollisionError); + registry.registerItem('page', { name: 'home', title: 'HR Home' }, 'name', 'com.acme.hr'), + ).not.toThrow(); }); - 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('keeps both packages reachable via package-scoped resolution', () => { + registry.registerItem('page', { name: 'home', title: 'CRM Home' }, 'name', 'com.acme.crm'); + registry.registerItem('page', { name: 'home', title: 'HR Home' }, 'name', 'com.acme.hr'); + expect(registry.getItem('page', 'home', 'com.acme.crm')?.title).toBe('CRM Home'); + expect(registry.getItem('page', 'home', 'com.acme.hr')?.title).toBe('HR Home'); }); - it('does NOT throw when the same package re-registers the same name (idempotent reload)', () => { - registry.registerItem('page', { name: 'home', title: 'v1' }, 'name', 'crm'); + it('does NOT error when the same package re-registers the same name (idempotent reload)', () => { + registry.registerItem('page', { name: 'home', title: 'v1' }, 'name', 'com.acme.crm'); expect(() => - registry.registerItem('page', { name: 'home', title: 'v2' }, 'name', 'crm'), + registry.registerItem('page', { name: 'home', title: 'v2' }, 'name', 'com.acme.crm'), ).not.toThrow(); // The latest value from the same package wins (overwrite under the same key). - expect(registry.getItem('page', 'home')?.title).toBe('v2'); + expect(registry.getItem('page', 'home', 'com.acme.crm')?.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. + it('keeps runtime/DB overlay (bare key) precedence over a packaged item (ADR-0005)', () => { + registry.registerItem('page', { name: 'home', title: 'packaged' }, 'name', 'com.acme.crm'); + // A runtime-authored row (no packageId) overlays it under the bare key. expect(() => registry.registerItem('page', { name: 'home', title: 'runtime' }, 'name'), ).not.toThrow(); + // Bare overlay wins even when a package id is supplied (documented precedence). + expect(registry.getItem('page', 'home', 'com.acme.crm')?.title).toBe('runtime'); }); - it('does NOT throw when a package ships over a pre-existing bare/runtime row', () => { - // Runtime/DB row registered first (no packageId)… + it('does NOT error when a package ships over a pre-existing bare/runtime row', () => { 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'), + registry.registerItem('page', { name: 'home', title: 'packaged' }, 'name', 'com.acme.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'. + it('does NOT error for the sys_metadata rehydration sentinel', () => { 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'), + registry.registerItem('page', { name: 'home', title: 'packaged' }, 'name', 'com.acme.crm'), ).not.toThrow(); }); - it('does NOT throw for different names across packages', () => { - registry.registerItem('page', { name: 'crm_home' }, 'name', 'crm'); + it('does NOT error for the same name across different types', () => { + registry.registerItem('page', { name: 'home' }, 'name', 'com.acme.crm'); expect(() => - registry.registerItem('page', { name: 'hr_home' }, 'name', 'hr'), + registry.registerItem('dashboard', { name: 'home' }, 'name', 'com.acme.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-prefer-local-resolution.test.ts b/packages/objectql/src/registry-prefer-local-resolution.test.ts index bcf964f668..383a449826 100644 --- a/packages/objectql/src/registry-prefer-local-resolution.test.ts +++ b/packages/objectql/src/registry-prefer-local-resolution.test.ts @@ -1,45 +1,32 @@ // Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. /** - * ADR-0048 Phase 2 — prefer-local (container-scoped) resolution. + * ADR-0048 §3.3 — package-scoped (prefer-local) resolution. * - * With the install-time namespace gate (Phase 1) keeping namespaces distinct, - * two packages may legitimately ship the same bare name (e.g. `page/home`). - * They no longer collide at registration; instead `getItem(type, name, ns)` - * routes each caller to the item owned by its own namespace's package. The - * per-item collision guard now fires only where prefer-local CANNOT - * disambiguate (shared / missing namespace). + * Two installed packages may ship the same bare name (e.g. `page/home`). They + * coexist under distinct composite keys, and `getItem(type, name, + * currentPackageId)` routes each caller to its own package's item. Because + * package ids are globally unique, this is unambiguous and needs no install-time + * gate to hold — it works for any caller carrying its package id. */ import { describe, it, expect, beforeEach } from 'vitest'; import { SchemaRegistry } from './registry'; -const install = (reg: SchemaRegistry, id: string, namespace: string) => - reg.installPackage({ id, name: id, namespace, version: '1.0.0' } as any); - -describe('SchemaRegistry — prefer-local resolution (ADR-0048 Phase 2)', () => { +describe('SchemaRegistry — package-scoped resolution (ADR-0048 §3.3)', () => { let registry: SchemaRegistry; beforeEach(() => { - registry = new SchemaRegistry({ multiTenant: false, collisionPolicy: 'error' }); + registry = new SchemaRegistry({ multiTenant: false }); registry.logLevel = 'silent'; - install(registry, 'com.acme.crm', 'crm'); - install(registry, 'com.acme.hr', 'hr'); }); - it('lets two distinct-namespace packages coexist on the same bare name', () => { - registry.registerItem('page', { name: 'home', title: 'CRM Home' }, 'name', 'com.acme.crm'); - expect(() => - registry.registerItem('page', { name: 'home', title: 'HR Home' }, 'name', 'com.acme.hr'), - ).not.toThrow(); - }); - - it('resolves prefer-local to the namespace owner', () => { + it('resolves prefer-local to the caller package', () => { registry.registerItem('page', { name: 'home', title: 'CRM Home' }, 'name', 'com.acme.crm'); registry.registerItem('page', { name: 'home', title: 'HR Home' }, 'name', 'com.acme.hr'); - expect(registry.getItem('page', 'home', 'crm')?.title).toBe('CRM Home'); - expect(registry.getItem('page', 'home', 'hr')?.title).toBe('HR Home'); + expect(registry.getItem('page', 'home', 'com.acme.crm')?.title).toBe('CRM Home'); + expect(registry.getItem('page', 'home', 'com.acme.hr')?.title).toBe('HR Home'); }); it('context-free getItem still returns one of the entries (legacy first-match fallback)', () => { @@ -52,28 +39,25 @@ describe('SchemaRegistry — prefer-local resolution (ADR-0048 Phase 2)', () => it('keeps runtime/DB overlay (bare key) precedence over prefer-local (ADR-0005)', () => { registry.registerItem('page', { name: 'home', title: 'CRM Home' }, 'name', 'com.acme.crm'); - // Runtime-authored overlay under the bare key (no package provenance). - registry.registerItem('page', { name: 'home', title: 'overlay' }, 'name'); + registry.registerItem('page', { name: 'home', title: 'overlay' }, 'name'); // bare, no package - expect(registry.getItem('page', 'home', 'crm')?.title).toBe('overlay'); + expect(registry.getItem('page', 'home', 'com.acme.crm')?.title).toBe('overlay'); }); - it('falls back to first-match when the namespace owns no such item', () => { + it('falls back to first-match when the caller package owns no such item', () => { registry.registerItem('page', { name: 'home', title: 'CRM Home' }, 'name', 'com.acme.crm'); - // hr has no `home`; asking within hr's container falls back to the only entry. - expect(registry.getItem('page', 'home', 'hr')?.title).toBe('CRM Home'); + // com.acme.hr has no `home`; asking within its package falls back to the only entry. + expect(registry.getItem('page', 'home', 'com.acme.hr')?.title).toBe('CRM Home'); }); - it('still fails loudly when two packages SHARE a namespace (unresolvable)', () => { - const reg = new SchemaRegistry({ multiTenant: false, collisionPolicy: 'error' }); - reg.logLevel = 'silent'; - // `sys` is a shareable platform namespace, exempt from the install gate, so - // two packages CAN both own it — but then a same-named item is ambiguous. - install(reg, 'com.a.sys', 'sys'); - install(reg, 'com.b.sys', 'sys'); - reg.registerItem('flow', { name: 'cleanup' }, 'name', 'com.a.sys'); - expect(() => - reg.registerItem('flow', { name: 'cleanup' }, 'name', 'com.b.sys'), - ).toThrow(); + it('disambiguates even when two packages share a namespace (package id is the key)', () => { + // `sys` is a shareable namespace, so two packages can both own it. Under + // package-scoped resolution that is still unambiguous — the key is the + // package id, not the namespace. + registry.registerItem('flow', { name: 'cleanup', title: 'A' }, 'name', 'com.a.sys'); + registry.registerItem('flow', { name: 'cleanup', title: 'B' }, 'name', 'com.b.sys'); + + expect(registry.getItem('flow', 'cleanup', 'com.a.sys')?.title).toBe('A'); + expect(registry.getItem('flow', 'cleanup', 'com.b.sys')?.title).toBe('B'); }); }); diff --git a/packages/objectql/src/registry.ts b/packages/objectql/src/registry.ts index acb2c81d0d..1431b8077d 100644 --- a/packages/objectql/src/registry.ts +++ b/packages/objectql/src/registry.ts @@ -143,19 +143,21 @@ export interface SchemaRegistryOptions { 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)`. + * Policy for the install-time namespace gate (ADR-0048 Phase 1) — installing + * a package whose `manifest.namespace` is already owned by a *different* + * installed package. * - * - `'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. + * - `'error'` (default): throw {@link NamespaceConflictError} at install time, + * naming both packages. Makes the namespace land-grab a loud, early failure + * instead of a mid-install `CREATE TABLE` blow-up. + * - `'warn'`: log a warning and let the install proceed. For deliberate + * migrations where the conflict 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. + * explicitly. Same-package reinstall and shareable platform namespaces + * (`base`/`system`/`sys`) are never treated as conflicts. (The per-item + * cross-package collision throw was retired in ADR-0048 §3.4 — distinct + * package ids are always disambiguable by package-scoped resolution.) */ collisionPolicy?: 'error' | 'warn'; } @@ -300,23 +302,6 @@ 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; -} - /** * Platform namespaces that multiple packages may legitimately share, so the * install-time namespace-uniqueness gate (ADR-0048 Phase 1) must never fire on @@ -328,46 +313,6 @@ function isShareableNamespace(ns: string): boolean { return RESERVED_NAMESPACES.has(ns) || ns === 'sys'; } -/** - * 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; - } -} - /** * Raised when a package is installed whose `manifest.namespace` is already owned * by a **different** installed package in this installation (ADR-0048 Phase 1). @@ -883,25 +828,16 @@ export class SchemaRegistry { // 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 && !this.isPreferLocalDisambiguable(packageId, conflictOwner)) { - // ADR-0048 Phase 2 — the guard now only fires when prefer-local - // resolution (see getItem) CANNOT disambiguate the two owners: they - // share a namespace (possible only for shareable platform namespaces - // like `sys`, which the install gate exempts) or one lacks a namespace - // (legacy package with no container to scope by). Two packages in - // DIFFERENT namespaces legitimately coexist on the same bare name — - // the install-time namespace gate keeps their namespaces distinct and - // prefer-local routes each caller to its own container. - const err = new MetadataCollisionError(type, baseName, conflictOwner, packageId); - if (this.collisionPolicy === 'warn') { - console.warn(`[Registry] ${err.message}`); - } else { - throw err; - } - } - } + // ADR-0048 §3.4 — the per-item CROSS-package throw is retired. Package ids + // are globally unique, so package-scoped resolution (see getItem) always + // disambiguates two different packages: two installed packages shipping the + // same bare name (e.g. `page/home`) legitimately COEXIST under distinct + // composite keys and each caller resolves to its own. What the original + // guard flagged as a collision is now the supported marketplace case. + // + // Same-package re-registration still overwrites (idempotent reload), and a + // runtime/DB overlay over a packaged item is the sanctioned ADR-0005 path, + // surfaced by the artifact-vs-DB warning below. // Artifact-vs-DB collision warning. When a code package ships an item // whose name already exists as a DB-only entry (registered earlier @@ -928,43 +864,6 @@ 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; - } - - /** - * True when prefer-local resolution ({@link getItem}) can route callers in - * each owner's container to the right item — i.e. the two packages declare - * **distinct, non-empty namespaces** (ADR-0048 Phase 2). When it returns - * false (shared namespace, or a missing namespace on either side) a same - * `(type, name)` clash is genuinely unresolvable and the collision guard - * fires. Namespaces are read from the installed-package records, which the - * install path registers before a package's metadata is loaded. - */ - private isPreferLocalDisambiguable(incoming: string, owner: string): boolean { - const incomingNs = this.getPackage(incoming)?.manifest?.namespace; - const ownerNs = this.getPackage(owner)?.manifest?.namespace; - return !!incomingNs && !!ownerNs && incomingNs !== ownerNs; - } - /** * Validate Metadata against Spec Zod Schemas */ @@ -1012,19 +911,21 @@ export class SchemaRegistry { /** * Universal Get Method. * - * ADR-0048 Phase 2 — *prefer-local* resolution. When `currentNamespace` is - * given (the container the caller is resolving within), a bare name resolves - * to the item owned by *that namespace's* package before any cross-package - * fallback, so two packages shipping e.g. `page/home` no longer resolve by - * registration order (first-match-wins). Omitting `currentNamespace` - * preserves the legacy resolution exactly, so this is backward compatible. + * ADR-0048 §3.3 — *package-scoped* resolution. When `currentPackageId` is + * given (the package the caller is resolving within — known from the route / + * `activeApp._packageId`), a bare name resolves to *that package's* item + * before any cross-package fallback, so two packages shipping e.g. + * `page/home` no longer resolve by registration order (first-match-wins). + * Because package ids are globally unique this is unambiguous. Omitting + * `currentPackageId` preserves the legacy resolution exactly (backward + * compatible) and is best-effort: it returns the first match. * * Precedence (highest first): * 1. bare-key runtime/DB overlay (ADR-0005 sanctioned override) — unchanged - * 2. the `currentNamespace` owner's composite entry (prefer-local) + * 2. the `currentPackageId` composite entry (prefer-local) * 3. first composite match (legacy first-registered-wins fallback) */ - getItem(type: string, name: string, currentNamespace?: string): T | undefined { + getItem(type: string, name: string, currentPackageId?: string): T | undefined { // Special handling for 'object' and 'objects' types - use objectContributors if (type === 'object' || type === 'objects') { return this.getObject(name) as unknown as T | undefined; @@ -1035,12 +936,10 @@ export class SchemaRegistry { const direct = collection.get(name); if (direct) return direct as T; - // Prefer-local: resolve within the caller's container (namespace) first. - if (currentNamespace) { - for (const owner of this.getNamespaceOwners(currentNamespace)) { - const local = collection.get(`${owner}:${name}`); - if (local) return local as T; - } + // Prefer-local: resolve within the caller's package first. + if (currentPackageId) { + const local = collection.get(`${currentPackageId}:${name}`); + if (local) return local as T; } // Fallback: first composite key matching the bare name (legacy behaviour). @@ -1290,11 +1189,11 @@ export class SchemaRegistry { this.registerItem('app', app, 'name', packageId); } - getApp(name: string): any { - // ADR-0048 (v1) — one app per package, with `app.name ≡ manifest.namespace`, - // so the app name *is* its own container: resolve prefer-local against it so - // two packages' apps never resolve by registration order. - const app = this.getItem('app', name, name); + getApp(name: string, currentPackageId?: string): any { + // ADR-0048 §3.1 — apps are addressed by package id (one app per package). + // When the caller knows it (route segment / `_packageId`), resolve + // prefer-local; otherwise this is a best-effort by-name lookup. + const app = this.getItem('app', name, currentPackageId); if (!app) return app; return this.applyNavContributions(app); }