fix(objectql): uninstalling a package removes the non-object metadata it shipped (#7221) - #7961
Conversation
… it shipped (#7221) `MetadataFacade.unregisterPackage` and `SchemaRegistry.uninstallPackage` both reached only `objectContributors`, so every non-object item a package shipped — `page`, `view`, `flow`, `app`, `api` … — stayed registered and fully resolvable after the package was gone, and the facade additionally orphaned the generic-map half of the package's objects. Measured before writing: after `uninstallPackage('crm')` the package record was gone while `getItem('page', 'home')` kept serving the uninstalled package's page and `metadata.get('flow')` still held `crm:onboard`. Both callers share the gap, which is why the new verb sits on `SchemaRegistry` rather than privately on the facade — a facade-local scan would have been a second expression of the same package-ownership rule and would have left the registry-direct caller half-done. `unregisterItemsByPackage(packageId)` is scoped to composite `${packageId}:${name}` keys, the exact inverse of `registerItem`'s construction. Bare-key ADR-0005 runtime/DB overlays are deliberately kept — they are tenant-authored and an uninstall does not delete tenant data — and the orphan that leaves behind is made loud rather than silently deleted or silently kept, following ADR-0029 D9.5. What nothing yet does with that report is filed as #7951. In both callers the sweep runs after the object verb, which can refuse on ADR-0029 extenders, so a refused uninstall removes nothing at all. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BtbjfNQKy7DXqNF7Y26vmP
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
📓 Docs Drift CheckThis PR changes 1 package(s): 15 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:
⛔ 1 release-owned page(s) also reference the affected code. These are read-only:
|
Closes #7221.
The defect
"Unregister all metadata from a package" reached only
objectContributors, so every non-object item a package shipped — itspage,view,flow,app,api… — stayed registered and fully resolvable after the package was gone. Not a stale-cache nuisance: an uninstall that leaves the package's UI and API metadata installed.A package writes into two stores.
unregisterObjectsByPackagewalks the contributor list; everything else lives in the genericmetadatamap under the composite${packageId}:${name}keyregisterItembuilds, and no verb removed those.MetadataFacade.unregisterPackageadditionally left the generic-map half of the package's objects behind as a genuine orphan (registerObjectBothPlaceswrites both halves; the object verb reaches one).Step 1 — the measurement that placed the verb
The card said the registry is the better home if
uninstallPackageshares the gap, and that this wanted measuring rather than assuming. Measured on a realSchemaRegistrybefore writing anything:crmunregisterPackageuninstallPackagepage/view/flowget,exists,listNamesall still serve themcrm:home,crm:onboardstill present;getItem('page','home')still serves['crm:contact']remainsSame gap. So the verb belongs on
SchemaRegistryand both callers get it (the card's disposition 1). A facade-private scan (disposition 2) would have been a second expression of the package-ownership rule — the #6808 drift — and would have left every registry-direct uninstall still half-done.What changed
SchemaRegistry.unregisterItemsByPackage(packageId)— new, in the generic-metadata section (well below PR #7851'sreconcileManagedApiMethodsseam at:672). Membership is the exact inverse ofregisterItem's construction, so the rule is expressed once: a key belongs to the package iff it starts with${packageId}:. A discriminated type's whole i18n bundle rides along at the end of the key and leaves with the package that shipped it; a scoped id (@acme/crm) works under the same relation.Both callers now sweep, in each case after the object verb — that one can refuse on ADR-0029 extenders, so a refused uninstall removes nothing at all:
MetadataFacade.unregisterPackageSchemaRegistry.uninstallPackageThe bare-key ruling, as applied
Took the non-destructive branch as ruled. The sweep is scoped to composite keys; ADR-0005 bare-key runtime/DB overlays are untouched. A bare key carries no package provenance and is the runtime face of a tenant-authored
sys_metadatarow, so deleting it would take a tenant's customization along with the package it merely overlaid.The precedents were re-verified by symbol on
main(8d80e12) and both still read as the ruling assumed:unregisterObjectsByPackage(packageId, force = false)(registry.ts:1730) refuses loudly, naming the extenders and telling the caller to uninstall those first — the caller decides, explicitly, viaforce.:1697) still reads "re-install the package that owns it, or delete thesys_metadatarow", and the D9.7 comment (:1758) still reads "an overlay layer leaves with the base it layers over".So the shape followed is identity-scoped removal + loudness about what it deliberately did not take: the verb
console.warns naming every overlay it orphaned and returns them asorphanedOverlaysfor a caller that wants to act.sys_metadatarow the removal does not touch, so a re-install re-hydrates it. A bare-key generic entry is the other way round — it is that row's runtime face, with no contributor list holding a durable copy — so the same rule would lose the tenant's edit.The consequence the ruling leaves open (nothing yet consumes the orphan report) is filed as #7951, not resolved here with a delete nobody authorised.
Reverse verification
Each behavioural change reverted alone, direction predicted before running.
get('page','home')returning the itemexpected { name: 'home', … } to be undefined; generic-map object half['crm:contact']vs[]; registry tests greenuninstallPackage's sweepgetItem('page','home'); refusal test stays greenconsole.warnwarnnot calledexpected "warn" to be called 1 times, but got 0 timesD is the one my prediction got wrong, and it found a gap in my own tests. The facade test I expected to fail registered
page/custom, which never overlays the removedcrm:home, so a destructive sweep cannot touch it — it pinned "an unrelated runtime item survives", not "a tenant's overlay survives". I added the missing case (a barepage/homeoverlaying the packagedcrm:homeat the facade seam) and re-ran D: 2 red as predicted, bothexpected undefined to match object { title: 'Tenant edit' }.The load-bearing defect is proven red by A: a package's
page/view/flowsurvivingunregisterPackageis what fails, not merely "something threw".Gates
All run in the worktree and watched:
pnpm --filter @objectstack/objectql testpnpm typecheckpnpm buildpnpm lintpnpm check:nul-bytespnpm check:error-code-casingpnpm check:empty-changesetpnpm check:query-options-erasure8d80e12, no files addedpnpm check:type-check-debtNeither ratchet baseline was raised and
--lowerwas not run — the surplus both report is pre-existing.Scope kept
17 tests in a new
registry-unregister-items-by-package.test.ts, plus a changeset.engine.tsuntouched (#7867 / #7922 seams),registry.ts:672untouched (PR #7851). The persistedsys_metadatarows (#7705) and the data-plane enforcement/envelope (#7557) are distinct mechanisms — read to keep all three faces of uninstall agreeing on what "uninstalled" means, but not reached into.🤖 Generated with Claude Code
https://claude.ai/code/session_01BtbjfNQKy7DXqNF7Y26vmP
Generated by Claude Code