Skip to content

fix(objectql): uninstalling a package removes the non-object metadata it shipped (#7221) - #7961

Merged
huangyiirene merged 1 commit into
mainfrom
claude/issue-7221-unregister-items-by-package
Aug 12, 2026
Merged

fix(objectql): uninstalling a package removes the non-object metadata it shipped (#7221)#7961
huangyiirene merged 1 commit into
mainfrom
claude/issue-7221-unregister-items-by-package

Conversation

@huangyiirene

Copy link
Copy Markdown
Collaborator

Closes #7221.

The defect

"Unregister all metadata from a package" reached only objectContributors, so every non-object item a package shipped — its page, 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. unregisterObjectsByPackage walks the contributor list; everything else lives in the generic metadata map under the composite ${packageId}:${name} key registerItem builds, and no verb removed those. MetadataFacade.unregisterPackage additionally left the generic-map half of the package's objects behind as a genuine orphan (registerObjectBothPlaces writes 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 uninstallPackage shares the gap, and that this wanted measuring rather than assuming. Measured on a real SchemaRegistry before writing anything:

after uninstalling crm facade unregisterPackage registry uninstallPackage
page / view / flow get, exists, listNames all still serve them keys crm:home, crm:onboard still present; getItem('page','home') still serves
generic-map half of objects ['crm:contact'] remains
contributor half / package record removed ✓ removed ✓

Same gap. So the verb belongs on SchemaRegistry and 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's reconcileManagedApiMethods seam at :672). Membership is the exact inverse of registerItem'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.unregisterPackage
  • SchemaRegistry.uninstallPackage

The 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_metadata row, 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, via force.
  • The ADR-0029 D9.5 orphan-overlay violation text (:1697) still reads "re-install the package that owns it, or delete the sys_metadata row", 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 as orphanedOverlays for a caller that wants to act.

⚠️ Deliberately not the object-side D9.7 rule. That rule is safe only because an object overlay layer is a runtime projection of a sys_metadata row 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.

revert predicted actual
A — drop the facade's sweep 4 facade tests red; the load-bearing one on get('page','home') returning the item ✅ 4 red, expected { name: 'home', … } to be undefined; generic-map object half ['crm:contact'] vs []; registry tests green
B — drop uninstallPackage's sweep exactly 1 red at getItem('page','home'); refusal test stays green ✅ 1 red, exactly there
C — drop the console.warn 1 red on warn not called expected "warn" to be called 1 times, but got 0 times
D — make the sweep destructive (delete bare keys) 2 red: registry bare-key test + facade tenant test ⚠️ 1 red, not 2 — see below

D 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 removed crm: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 bare page/home overlaying the packaged crm:home at the facade seam) and re-ran D: 2 red as predicted, both expected undefined to match object { title: 'Tenant edit' }.

The load-bearing defect is proven red by A: a package's page/view/flow surviving unregisterPackage is what fails, not merely "something threw".

Gates

All run in the worktree and watched:

gate result
pnpm --filter @objectstack/objectql test ✅ exit 0 — 189 files, 3348 tests
pnpm typecheck ✅ exit 0 — 126 tasks
pnpm build ✅ exit 0 — 71 tasks
pnpm lint ✅ exit 0
pnpm check:nul-bytes ✅ exit 0 — 7281 files
pnpm check:error-code-casing ✅ exit 0
pnpm check:empty-changeset ✅ exit 0 — 1 declaring changeset added
pnpm check:query-options-erasure ✅ exit 0 — baseline verified against 8d80e12, no files added
pnpm check:type-check-debt ✅ exit 0 — 36 entries re-measured, none above its recorded number

Neither ratchet baseline was raised and --lower was 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.ts untouched (#7867 / #7922 seams), registry.ts:672 untouched (PR #7851). The persisted sys_metadata rows (#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

… 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
@vercel

vercel Bot commented Aug 12, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
objectstack Ignored Ignored Aug 12, 2026 8:58am

Request Review

@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/objectql.

15 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:

  • content/docs/concepts/metadata-lifecycle.mdx (via @objectstack/objectql)
  • content/docs/data-modeling/formulas.mdx (via packages/objectql)
  • content/docs/deployment/migration-from-objectql.mdx (via @objectstack/objectql)
  • content/docs/deployment/vercel.mdx (via @objectstack/objectql)
  • content/docs/kernel/contracts/data-engine.mdx (via @objectstack/objectql)
  • content/docs/kernel/runtime-services/examples.mdx (via packages/objectql)
  • content/docs/kernel/services-checklist.mdx (via @objectstack/objectql)
  • content/docs/kernel/services.mdx (via @objectstack/objectql)
  • content/docs/permissions/authentication.mdx (via @objectstack/objectql)
  • content/docs/permissions/system-context.mdx (via packages/objectql)
  • content/docs/plugins/index.mdx (via @objectstack/objectql)
  • content/docs/plugins/packages.mdx (via @objectstack/objectql)
  • content/docs/protocol/kernel/index.mdx (via @objectstack/objectql)
  • content/docs/protocol/objectql/query-syntax.mdx (via packages/objectql)
  • content/docs/protocol/objectql/state-machine.mdx (via @objectstack/objectql)

1 release-owned page(s) also reference the affected code. These are read-only:

  • content/docs/releases/implementation-status.mdx (via @objectstack/objectql)

content/docs/releases/ is RELEASE-OWNED (AGENTS.md "Documentation Guardrails"): release
notes are written centrally at release time, and a code PR that edits them is the exact PR
that guardrail exists to stop. They are still audited — read-only. If one of them is actually
wrong, file an issue or open a dedicated docs-only PR; do not edit it here.

Advisory only. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs origin/main → pass the list as args.docs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size/l tests tooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

MetadataFacade.unregisterPackage removes only object contributors — every non-object item the package shipped stays registered

2 participants