|
| 1 | +# ADR-0048: Cross-package metadata collision — detect, don't silently overwrite |
| 2 | + |
| 3 | +**Status**: Proposed (2026-06-13) |
| 4 | +**Deciders**: ObjectStack Protocol Architects |
| 5 | +**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) |
| 6 | +**Consumers**: `@objectstack/objectql` (`SchemaRegistry.registerItem`, `ObjectQL.registerApp`), package authors, CLI/CI install path |
| 7 | +**Surfaced by**: ADR-0046 review (doc naming) — generalised here into its own work item. |
| 8 | + |
| 9 | +--- |
| 10 | + |
| 11 | +## TL;DR |
| 12 | + |
| 13 | +The metadata registry key is `org/type/name` — it has **no package |
| 14 | +coordinate** (`refKey` in `packages/metadata-core/src/types.ts`). Object |
| 15 | +names dodge collisions because the kernel namespace-prefix-validates them |
| 16 | +(they map to physical table names). But **bare-named UI/automation metadata |
| 17 | +is not prefix-validated**: `page`, `dashboard`, `flow`, `app`, `action`, |
| 18 | +`doc` only require snake_case. So two installed packages that each define a |
| 19 | +`page` named `home` produce the same logical key, and the second |
| 20 | +registration **silently shadows the first** — worse than the object case, |
| 21 | +which fails loudly at the DB. |
| 22 | + |
| 23 | +**Decision:** detect cross-package same-key collisions in the code-defined |
| 24 | +**base layer** at registration time and raise an explicit, actionable error |
| 25 | +naming both packages and the type/name. Do **not** retrofit |
| 26 | +namespace-prefix enforcement onto every existing bare-named type (large |
| 27 | +migration cost). Prefix stays a *recommended convention* — and brand-new |
| 28 | +types can be strict from day one. |
| 29 | + |
| 30 | +## 1. Context |
| 31 | + |
| 32 | +### 1.1 The registry key carries no package coordinate |
| 33 | + |
| 34 | +Metadata identity is `(org, type, name)`: |
| 35 | + |
| 36 | +```ts |
| 37 | +// packages/metadata-core/src/types.ts |
| 38 | +export function refKey(ref: Pick<MetaRef, 'org' | 'type' | 'name'>): string { |
| 39 | + return `${ref.org}/${ref.type}/${ref.name}`; |
| 40 | +} |
| 41 | +``` |
| 42 | + |
| 43 | +Nothing in that key says *which package* a `system/page/home` came from. |
| 44 | +For objects this is harmless: object names are validated against a |
| 45 | +namespace prefix in the kernel (`validateNamespacePrefix` in |
| 46 | +`packages/spec/src/stack.zod.ts`) because they become physical table names, |
| 47 | +so two packages cannot both ship `account` — and if they tried, the second |
| 48 | +`CREATE TABLE` fails **loudly** at the database. |
| 49 | + |
| 50 | +Bare-named UI/automation metadata has no such backstop. `page`, |
| 51 | +`dashboard`, `flow`, `app`, `action`, and (as of ADR-0046) `doc` only |
| 52 | +require `SnakeCaseIdentifierSchema`. Two packages can each legitimately |
| 53 | +declare a `page` named `home`. |
| 54 | + |
| 55 | +### 1.2 How the silent shadowing actually happens |
| 56 | + |
| 57 | +In the objectql `SchemaRegistry`, generic (non-object) metadata lives in a |
| 58 | +two-level map and is stored under a **composite** key when a package id is |
| 59 | +present: |
| 60 | + |
| 61 | +```ts |
| 62 | +// packages/objectql/src/registry.ts — registerItem() |
| 63 | +const storageKey = packageId ? `${packageId}:${baseName}` : baseName; |
| 64 | +collection.set(storageKey, item); |
| 65 | +``` |
| 66 | + |
| 67 | +So `crm` and `hr` both shipping `page/home` do **not** overwrite the same |
| 68 | +map entry — they sit under `crm:home` and `hr:home`. The shadowing surfaces |
| 69 | +one layer up, at **read** time: |
| 70 | + |
| 71 | +```ts |
| 72 | +// getItem() — returns the FIRST composite key matching `:<name>` |
| 73 | +for (const [key, item] of collection) { |
| 74 | + if (key.endsWith(`:${name}`)) return item as T; |
| 75 | +} |
| 76 | +``` |
| 77 | + |
| 78 | +`getItem('page', 'home')` returns whichever entry the `Map` iterates first |
| 79 | +— i.e. **whichever package was registered first**. The other package's |
| 80 | +`home` is unreachable by name, with no error and no warning. It is |
| 81 | +last-write-wins (here, *first-registered-wins*) and entirely silent — the |
| 82 | +exact failure ADR-0046's review flagged for `doc`, generalised to every |
| 83 | +bare-named type. |
| 84 | + |
| 85 | +### 1.3 What is *not* a collision (and must keep working) |
| 86 | + |
| 87 | +The same `(type, name)` is written more than once for entirely legitimate |
| 88 | +reasons. The guard must not break these: |
| 89 | + |
| 90 | +- **Same-package reload.** Re-registering a package (dev reload, idempotent |
| 91 | + install) re-writes `crm:home` with `crm`'s own value. Same owner — not a |
| 92 | + collision. |
| 93 | +- **Runtime / DB overlay (ADR-0005).** A runtime-authored row in |
| 94 | + `sys_metadata` overlays a packaged artifact. It is registered under the |
| 95 | + **bare** key with no real package provenance (or carries the |
| 96 | + `'sys_metadata'` rehydration sentinel as `_packageId`). This is the |
| 97 | + sanctioned override path; `registerItem` already emits an artifact-vs-DB |
| 98 | + *shadowing warning* for it and must continue to allow it. |
| 99 | +- **Object ownership / extension.** Objects use a separate |
| 100 | + contributor model (`own` / `extend`, `registerObject`) and never flow |
| 101 | + through this guard. |
| 102 | +- **Navigation contributions (ADR-0029).** A package injecting nav items |
| 103 | + into an app it does not own uses `appNavContributions`, not a duplicate |
| 104 | + `app` registration. |
| 105 | + |
| 106 | +The bug is specifically a **base-layer collision between two different code |
| 107 | +packages**. Provenance is already available to tell them apart: |
| 108 | +ADR-0010 stamps every artifact-registered item with `_packageId` |
| 109 | +(`applyProtection`), and the registration call passes the owning package id |
| 110 | +explicitly. |
| 111 | + |
| 112 | +## 2. Goals & non-goals |
| 113 | + |
| 114 | +**Goals** |
| 115 | +- Make a cross-package base-layer collision a loud, actionable failure at |
| 116 | + registration/install time, naming both packages and the type/name. |
| 117 | +- Cost-cheap: piggyback on the registration path, which already reads the |
| 118 | + collection by key. |
| 119 | +- Zero false positives on overlays, same-package reloads, objects, and nav |
| 120 | + contributions. |
| 121 | + |
| 122 | +**Non-goals** |
| 123 | +- Retrofitting namespace-prefix enforcement onto existing bare-named types |
| 124 | + (`page`, `flow`, …). That is a breaking rename for every shipped package |
| 125 | + and is out of scope. |
| 126 | +- Changing the `org/type/name` key shape or adding a package column to |
| 127 | + `sys_metadata`. |
| 128 | +- Cross-**org** overlay semantics (unchanged; ADR-0005 governs them). |
| 129 | + |
| 130 | +## 3. Decision |
| 131 | + |
| 132 | +### 3.1 Detect, error, name the culprits |
| 133 | + |
| 134 | +At registration time, when a code package registers a bare-named generic |
| 135 | +item, refuse it if a **different** code package already owns the same |
| 136 | +`(type, name)` in the base layer. The error names both packages, the type, |
| 137 | +and the name, and points at the fix. |
| 138 | + |
| 139 | +Detection lives at the single choke point that every installed package's |
| 140 | +metadata arrays pass through — `SchemaRegistry.registerItem` |
| 141 | +(`packages/objectql/src/registry.ts`). `ObjectQL.registerApp` (and the |
| 142 | +nested-plugin loop) delegate to it, so guarding it once covers manifest |
| 143 | +metadata and plugin metadata alike. The check: |
| 144 | + |
| 145 | +> `registerItem` is called with a real `packageId`, **and** an existing |
| 146 | +> entry for the same `(type, name)` carries a *different* real `_packageId` |
| 147 | +> (truthy, and not the `'sys_metadata'` sentinel) → `MetadataCollisionError`. |
| 148 | +
|
| 149 | +Same-package writes (`owner === incoming`), bare/overlay rows (no real |
| 150 | +owner), and the `'sys_metadata'` sentinel are all excluded, so the |
| 151 | +legitimate cases in §1.3 pass through untouched. Detection scans the live |
| 152 | +collection — exactly as `getItem`/`unregisterItem` already do — so there is |
| 153 | +no parallel index to drift across `reset`/`unregister`. |
| 154 | + |
| 155 | +### 3.2 Policy is `error` by default, `warn` as an escape hatch |
| 156 | + |
| 157 | +`collisionPolicy` defaults to `'error'`. A `'warn'` mode (constructor option |
| 158 | +or `OS_METADATA_COLLISION=warn`) downgrades to a logged warning and lets the |
| 159 | +registration proceed, for deliberate, temporary migrations (e.g. renaming a |
| 160 | +colliding page across two packages in flight). The default is loud; the |
| 161 | +opt-out is explicit and discoverable from the error message itself. |
| 162 | + |
| 163 | +### 3.3 Why detection over prefix enforcement |
| 164 | + |
| 165 | +Two ways to kill the collision: |
| 166 | + |
| 167 | +1. **Prefix enforcement** — require every bare-named type's `name` to start |
| 168 | + with the package namespace, like objects. Closes the hole at the source, |
| 169 | + but renames the entire installed base (every `page`/`flow`/`action` in |
| 170 | + every shipped package and pilot), breaks cross-references, and forces a |
| 171 | + coordinated migration. High cost, high blast radius. |
| 172 | +2. **Collision detection** (this ADR) — leave existing names alone; make the |
| 173 | + *clash* an error. Near-zero migration cost, the registration path already |
| 174 | + reads the key, and the failure is actionable. |
| 175 | + |
| 176 | +We choose (2). Prefixing remains the **recommended convention** — the error |
| 177 | +message literally suggests `<namespace>_<name>` — and may be surfaced as a |
| 178 | +non-fatal *lint warning* in the CLI later, but it is **not retroactively |
| 179 | +enforced** on legacy types. |
| 180 | + |
| 181 | +### 3.4 New types can be strict from day one |
| 182 | + |
| 183 | +A type introduced *after* this ADR has no installed base to migrate, so it |
| 184 | +can adopt namespace-prefix validation immediately at the spec layer and get |
| 185 | +both guarantees (no collision *and* self-describing names). ADR-0046's |
| 186 | +`doc` is the first candidate: its CLI already enforces namespace-prefixed |
| 187 | +snake_case names at build time, so `doc` is effectively prefix-strict at the |
| 188 | +authoring boundary while this ADR's registry guard is its runtime backstop. |
| 189 | +The general rule: **legacy types → detect; new types → prefix-strict + |
| 190 | +detect.** |
| 191 | + |
| 192 | +## 4. Consequences |
| 193 | + |
| 194 | +- A genuine cross-package clash now fails fast at boot/install with a |
| 195 | + message identifying both packages — instead of a coin-flip over which |
| 196 | + package's `home` page a user sees. The hazard moves from *silent at read* |
| 197 | + to *loud at registration*. |
| 198 | +- Package authors who unknowingly relied on first-registered-wins will get |
| 199 | + an error; the fix (rename with a namespace prefix, or `warn` during |
| 200 | + migration) is in the message. |
| 201 | +- No change to the key shape, the overlay model, or object/nav paths. |
| 202 | +- Follow-ups (not in this ADR): a CLI lint that flags non-prefixed |
| 203 | + bare-named metadata as a warning; prefix-strict spec validation for the |
| 204 | + next net-new bare-named type. |
| 205 | + |
| 206 | +## 5. Implementation notes |
| 207 | + |
| 208 | +- `packages/objectql/src/registry.ts`: `MetadataCollisionError` (exported), |
| 209 | + `isRealPackage` helper (excludes the `'sys_metadata'` sentinel), |
| 210 | + `collisionPolicy` option + `OS_METADATA_COLLISION` env, the guard in |
| 211 | + `registerItem`, and `findOtherPackageOwner` (live-collection scan). |
| 212 | +- Tests: `registry-cross-package-collision.test.ts` (unit — error/warn, |
| 213 | + same-package reload, overlay, sentinel, distinct names) and |
| 214 | + `engine-cross-package-collision.test.ts` (end-to-end through |
| 215 | + `ObjectQL.registerApp`). |
| 216 | +- The `metadata-core` repository (`refKey`, `put`) is the *conceptual* root |
| 217 | + of the missing package coordinate, but its optimistic-concurrency |
| 218 | + `parentVersion` check already rejects a blind base-layer double-create |
| 219 | + with `ConflictError`; the genuinely *silent* path is the objectql |
| 220 | + `SchemaRegistry` read resolution, which is where enforcement lands. |
0 commit comments