From beaf38d2ab15f77182a0cd0f97c4fc7f15c482e8 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 1 Nov 2026 18:54:01 +0000 Subject: [PATCH 1/5] fix(objectql): a package refused part-way through registering leaves nothing of itself registered Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_01FEkQ5kPpaFpvn6zyWoBxLN --- packages/objectql/src/engine.ts | 531 +++++++++++++----- ...age-refused-registration-withdrawn.test.ts | 246 ++++++++ packages/objectql/src/registry.ts | 15 +- 3 files changed, 649 insertions(+), 143 deletions(-) create mode 100644 packages/objectql/src/package-refused-registration-withdrawn.test.ts diff --git a/packages/objectql/src/engine.ts b/packages/objectql/src/engine.ts index 76f6b7507cc..67850752153 100644 --- a/packages/objectql/src/engine.ts +++ b/packages/objectql/src/engine.ts @@ -331,7 +331,10 @@ import { } from './home-amount-projection.js'; import { assertAggregatedColumnsAreDistinct, assertDateBucketTargetsAreTemporal } from './aggregate-query-shape-door.js'; import { applyProtection, pluralToSingular, ExternalWriteForbiddenError } from '@objectstack/spec/shared'; -import { SchemaRegistry, type ArtifactInstallScope } from './registry.js'; +import { SchemaRegistry, computeFQN, type ArtifactInstallScope } from './registry.js'; +// [#28697] Whether an object's sitting owner is a tenant's own (ADR-0029 D9.6), +// asked by `refusedRegistrationWithdrawable` before a refused body is withdrawn. +import { isTenantAuthored } from '@objectstack/metadata-core'; import { expandSearchToFilter, normalizeSearch, SEARCHABLE_ENUM_TYPES, SEARCHABLE_TEXTUAL_TYPES } from './search-filter.js'; import { stripSearchCompanion } from './search-companion.js'; import { ExpressionEngine, analyzeRelationshipTraversals, isUnparseableDate, type EvalError, type EvalOptionLabels, type EvalPermissions } from '@objectstack/formula'; @@ -4986,6 +4989,254 @@ export function packageBodyObjectAndAppOwnerStampRefusal( return undefined; } +/** + * [#28697] Every refusal the collection seam (`registerMetadataCollections`) + * raises that the body alone decides, asked over a package body or a nested + * plugin BEFORE anything of the package registers. The first refusal found, + * walking the collections and items in the order the seam registers them, or + * `undefined`. + * + * The seam used to raise each of these on the item it was about to register, + * after `installPackage`, the objects and the apps had registered: the + * refusal propagated (install-local answered `422 PLUGIN_REGISTER_FAILED`) + * while the package record, objects and apps stayed in the live registry until + * restart, and a refused re-register of an installed id left that install + * half-replaced. Every judge here reads the item, its derived name, the + * source label and the owning id, and nothing the registry holds, so asking + * it first gives the same answer in the same words. ⛔ No second copy of any + * judge: each is the one function its own record names. + * + * Called by {@link ObjectQL.registerApp} for the body (`'manifest'`) and for + * each nested plugin (`'nested plugin'`, under the parent's id, as + * `registerPlugin` registers it). + */ +export function packageBodyCollectionRefusal( + source: any, + ownerId: string, + sourceLabel: string, +): Error | undefined { + for (const key of METADATA_ARRAY_KEYS) { + // A `picklistExtensions` entry is not an item (the seam hands it to + // `registerPicklistExtension`, whose refusal reads the registry). + if (key === 'picklistExtensions') continue; + const items = (source as any)?.[key]; + if (!Array.isArray(items) || items.length === 0) continue; + for (const item of items) { + const itemName = resolveMetadataItemName(key, item); + // The seam skips (and warns about) an item with no derivable name. + if (!itemName) continue; + // [commit d0ee598e6] The DIVERGENT-container refusal (maintainer ruling + // 2026-09-03, direction 2). The collection seam used to reconcile a + // container's own `name` to the derived key silently: the author's + // field discarded with no diagnostic, while + // the OTHER source registrar + // (`MetadataPlugin._parseAndRegisterArtifact` -> + // `assertMetadataRegisterContract`, #7378 row 1) hard-failed the + // whole artifact load on that very same document. One document, + // two SOURCE registrars, opposite outcomes, and which one an + // author got depended on how their package was loaded. #7378 + // row 1 already rules that resolving a name/key disagreement + // silently "in either direction" files the item under a key the + // caller never wrote, so the boot loop converges onto the + // refusal; the artifact door was not moved onto the rewrite. + // + // MEASURED, and it is why only this site changed: the second + // `toRegister` ternary in the seam (the assembled + // `viewItems:` channel below) cannot carry a container at all — + // `AssembledViewArtifactSchema` is the view vocabulary MINUS the + // container branch, and every body it admits is + // `isAggregatedViewContainer === false`. Its ternary can only + // MINT a `name` onto an overlay that has none; it can never + // discard an authored one, because `resolveMetadataItemName` + // reads a non-container's own `name` FIRST. + // + // Scope is the ruling's named main risk, so the gate is three + // independent narrowings and each one is load-bearing. The first + // stays here: + // * `key === 'views'` — the seam is the GENERIC metadata loop; no + // other kind changes behaviour and they all keep the + // reconcile. + // The other two — the CONTAINER branch only, and `name` present + // AND different — and the message, the envelope and the reason + // the prose is this seam's own live in `viewContainerNameRefusal` + // (`view-container-name-refusal.ts`). + // + // [#20331] Moved there so that `os validate` runs the SAME + // judgment, in the same words, instead of passing a document this + // loop then refuses at boot. The message and the envelope moved + // unchanged. The `!itemName` skip just above is this loop's + // precondition for the check, so the function carries it too: it + // answers `undefined` for a falsy derived key, and a door calling + // it without this loop cannot refuse an entry this loop skips. + // ⛔ Do not re-inline it here, and do not write a second copy of + // it anywhere else: one judge, two doors. + if (key === 'views') { + const refusal = viewContainerNameRefusal(item, sourceLabel, ownerId); + if (refusal) return refusal; + } + // [ADR-0086 D3] A permission set whose body `packageId` disagrees + // with the package the seam registers it under is refused, in + // the same envelope as the container refusal above: + // `registerItem` stamps `ownerId` as the set's `_packageId`, the + // owner every reader reads, so the body's key used to be + // registered over in silence. See + // {@link permissionSetOwnerRegistrationRefusal}. + if (key === 'permissions') { + const refusal = permissionSetOwnerRegistrationRefusal(item, itemName, ownerId); + if (refusal) return refusal; + } + // [#28534] …and an item of ANY collection whose body already wears + // a `_packageId` other than `ownerId`: `stampPackage` and + // `registerItem` stamp the owner only while the key is unset, so + // the body's own stamp used to win. See + // {@link collectionItemOwnerStampRefusal}. + { + const refusal = collectionItemOwnerStampRefusal(item, pluralToSingular(key), itemName, ownerId); + if (refusal) return refusal; + } + // [#5320] The `views:` tighten — containers ONLY, the contract the + // stack schema has always declared (`stack.zod.ts`, + // `z.array(ViewSchema)`). The seam used to register EVERY entry + // as type `view` — the "runtime wider than schema" hole #5320 + // records: a ViewItem or flattened overlay that `defineStack` + // refuses registered there in silence. Judged by the SAME + // classifier the manifest assemblers partition with + // (`isViewContainerShaped`), so a produced manifest cannot carry + // a `views:` entry the seam refuses. Deliberately a SHAPE-CLASS + // judgement, not a full `ViewSchema.parse`: which class an entry + // belongs to is the seam's contract; whether a container's + // internals are well-formed stays the authoring/publish doors' + // job (defineStack, `os validate`, the metadata door). + // The envelope and the words: {@link viewsEntryNotContainerRefusal}, + // which the residual registration throws too. Judged on the body the + // seam registers (its `name` reconciled to the derived key). + if (key === 'views') { + const toRegister = item.name === itemName ? item : { ...item, name: itemName }; + if (!isViewContainerShaped(toRegister)) return viewsEntryNotContainerRefusal(itemName, sourceLabel, ownerId); + } + } + } + // [#5320] The `viewItems:` channel, each entry judged by its schema and + // the PARSED body's owner stamp (#28534), as the seam registers it. + const assembledItems = (source as any)?.[ASSEMBLED_VIEW_ITEMS_KEY]; + if (Array.isArray(assembledItems)) { + for (const item of assembledItems) { + const parsed = AssembledViewArtifactSchema.safeParse(item); + if (!parsed.success) return assembledViewItemRefusal(item, parsed.error, sourceLabel, ownerId); + const body = parsed.data as Record; + const itemName = resolveMetadataItemName('views', body); + if (!itemName) continue; + const refusal = collectionItemOwnerStampRefusal(body, 'view', itemName, ownerId); + if (refusal) return refusal; + } + } + return undefined; +} + +/** + * [#28697] The seed step's owner-stamp refusal ({@link collectionItemOwnerStampRefusal}, + * #28534: the one collection filed outside the collection seam), asked over a + * package body's `data` datasets before anything of the package registers. + * The first refusal found, or `undefined`. Called by {@link ObjectQL.registerApp}. + */ +export function packageBodySeedDataRefusal(manifest: any, ownerId: string): Error | undefined { + const seedData = manifest?.data; + if (!Array.isArray(seedData)) return undefined; + for (const dataset of seedData) { + // The seed step registers a dataset with a target object only. + if (!dataset?.object) continue; + const refusal = collectionItemOwnerStampRefusal(dataset, 'data', String(dataset.object), ownerId); + if (refusal) return refusal; + } + return undefined; +} + +/** + * [#5320] The refusal of a `viewItems:` entry its schema + * (`AssembledViewArtifactSchema`) does not admit: `INVALID_METADATA` / 422, + * naming the entry and its source. Asked by {@link packageBodyCollectionRefusal} + * and by the collection seam, which parses the entry again for the body it + * registers. ⛔ Never a second copy of the text. + */ +export function assembledViewItemRefusal( + item: unknown, + error: { issues: ReadonlyArray<{ message: string }> }, + sourceLabel: string, + ownerId: string | undefined, +): Error & { code: 'INVALID_METADATA'; status: 422; httpStatus: 422 } { + const itemName = resolveMetadataItemName('views', item) ?? '(unnamed)'; + const err = new Error( + `Invalid \`${ASSEMBLED_VIEW_ITEMS_KEY}:\` entry '${itemName}' from ${sourceLabel} '${ownerId}': ` + + 'the assembled-manifest channel carries non-container view artifacts only — a ViewItem ' + + 'record (`viewKind` + `config`) or a flattened list/form overlay ' + + '(`AssembledViewArtifactSchema`, @objectstack/spec). A view CONTAINER travels in `views:`. ' + + `First issue: ${error.issues[0]?.message ?? 'no issue detail'}`, + ) as Error & { code: 'INVALID_METADATA'; status: 422; httpStatus: 422 }; + err.code = 'INVALID_METADATA'; + err.status = 422; + err.httpStatus = 422; + return err; +} + +/** + * [#28697] Whether a refused {@link ObjectQL.registerApp} of package `id` may + * be withdrawn through the registry's uninstall, asked BEFORE anything of the + * body is written. True only when withdrawing takes back exactly what the + * refused call wrote and nothing else: + * + * - no package record answers `id` yet. A refused re-register of an + * installed id is NOT withdrawn: the uninstall would take the installed + * version with it; + * - `id` is not the owner a stack's residual registers its items under + * (`recordResidualOwner`), whose items no package record covers; + * - no object the body or a nested plugin owns, nor any object it extends, + * already holds a contribution under `id`; + * - every object the body or a nested plugin owns is either unheld or owned + * by another code package (which `registerObject` refuses before it adds + * anything). A tenant-authored owner or an overlay or extension with no + * owner is left alone: `registerObject` re-classifies the tenant + * contribution as the package's overlay, and the uninstall drops an + * object's overlay with its owner (ADR-0029 D9.7) and refuses an object + * another package extends, so withdrawing would lose the tenant's layer in + * this process or be refused. + */ +export function refusedRegistrationWithdrawable( + registry: SchemaRegistry, + id: string | undefined, + manifest: any, + residualOwners: ReadonlyMap, +): boolean { + if (!id || registry.getPackage(id) !== undefined || residualOwners.has(id)) return false; + // The object names as the registry files them (`computeFQN`, the call + // `registerObject` makes with the namespace each one registers under). + const owned: string[] = []; + const ownedNames = (body: any, namespace: string | undefined) => { + const objects = body?.objects; + if (!objects || typeof objects !== 'object') return; + const names = Array.isArray(objects) ? objects.map((def: any) => def?.name) : Object.keys(objects); + for (const name of names) if (typeof name === 'string' && name !== '') owned.push(computeFQN(namespace, name)); + }; + ownedNames(manifest, manifest?.namespace); + if (Array.isArray(manifest?.plugins)) { + for (const plugin of manifest.plugins) { + if (plugin && typeof plugin === 'object') ownedNames(plugin, plugin.namespace || manifest?.namespace); + } + } + const extended: string[] = Array.isArray(manifest?.objectExtensions) + ? manifest.objectExtensions.map((ext: any) => ext?.extend).filter((name: unknown): name is string => typeof name === 'string') + : []; + for (const fqn of [...owned, ...extended]) { + if (registry.getObjectContributors(fqn).some((c) => c.packageId === id)) return false; + } + for (const fqn of owned) { + const contributors = registry.getObjectContributors(fqn); + if (contributors.length === 0) continue; + const ownedByAnotherCodePackage = contributors.some((c) => c.ownership === 'own' && !isTenantAuthored(c.definition)); + if (!ownedByAnotherCodePackage) return false; + } + return true; +} + /** * [#28697] Stamp an app object ({@link stampPackage}) and register it through * the registry's app verb, recording the stamp the registration writes onto @@ -9050,10 +9301,43 @@ export class ObjectQL implements IObjectQLEngine { if (nested) throw nested; } } + // [#28697] …and every refusal the collection seam (steps 5 and 7) + // and the seed step (step 6) can judge from the body alone, so a + // body refused there leaves nothing of itself behind either, and a + // refused re-register of an installed id leaves that install as it + // was. See {@link packageBodyCollectionRefusal}. + const collections = packageBodyCollectionRefusal(manifest, id, 'manifest'); + if (collections) throw collections; + const seed = packageBodySeedDataRefusal(manifest, id); + if (seed) throw seed; + if (Array.isArray(manifest.plugins)) { + for (const plugin of manifest.plugins) { + if (!plugin || typeof plugin !== 'object') continue; + const nested = packageBodyCollectionRefusal(plugin, id, 'nested plugin'); + if (nested) throw nested; + } + } } this.invalidateSummaryIndex(); // new objects may add/change summary fields this.logger.debug('Registering package manifest', { id, namespace }); + // [#28697] What remains can only be refused part-way, by the registry's + // own verbs (an object another package owns, a field with no `type`, a + // picklist value already added, …). A first registration of this id is + // withdrawn when one throws, so the caller's refusal leaves no package + // record, object, app or item behind; asked here, before anything is + // written. See {@link refusedRegistrationWithdrawable}. + const withdrawable = refusedRegistrationWithdrawable(this._registry, id, manifest, this.residualOwnerSources); + try { + this.registerPackageBody(manifest, id, namespace, version, source, scope); + } catch (err) { + if (withdrawable) this.withdrawRefusedRegistration(id, manifest, err); + throw err; + } + + // [#28697] Written only once nothing above threw, so a refused body + // leaves none of them. Nothing above reads any of them. + // Store manifest for defaultDatasource lookup if (id) { this.manifests.set(id, manifest); @@ -9071,6 +9355,55 @@ export class ObjectQL implements IObjectQLEngine { } } + // Register navigation contributions (ADR-0029 D7) — nav items this + // package injects into apps owned by other packages (e.g. a capability + // plugin adding its menu into the `setup` app). Merged into the target + // app's navigation on read by group id + priority, so registration order + // does not matter. + if (Array.isArray((manifest as any).navigationContributions) && (manifest as any).navigationContributions.length > 0) { + for (const contribution of (manifest as any).navigationContributions) { + this._registry.registerAppNavContribution(contribution, id); + } + this.logger.debug('Registered navigation contributions', { + from: id, + count: (manifest as any).navigationContributions.length, + }); + } + + // Register contributed kinds (owned by no package, so written last). + if (manifest.contributes?.kinds) { + this.logger.debug('Registering kinds from manifest', { id, kindCount: manifest.contributes.kinds.length }); + for (const kind of manifest.contributes.kinds) { + this._registry.registerKind(kind); + // [commit 10485009a] Name the kind by its declared `id`. `contributes.kinds` + // items are `{ id, description? }` (`manifest.zod.ts`; `globs` was + // retired unread, #11169) and + // `registerKind` keys the item on `id` (`registerItem('kind', kind, 'id')`), + // so `id` is BOTH the only identifying field the schema declares and the + // exact key the item is stored under — a reader of this line can look the + // item straight back up. The previous `kind.name || kind.type` reached for + // two fields NEITHER shape declares, so every conforming manifest logged + // `kind: undefined`. Do not re-add those as a fallback: reading undeclared + // aliases here is the consumer-side tolerance Prime Directive #12 rejects. + this.logger.debug('Registered Kind', { kind: kind.id, from: id }); + } + } + } + + /** + * [#28697] The part of {@link registerApp} a registry verb can refuse + * part-way: the package record, its objects, extensions, apps, collections, + * seed data and nested plugins, in that order. Every body-only refusal was + * judged before this runs; what is written after it is in `registerApp`. + */ + private registerPackageBody( + manifest: any, + id: string, + namespace: string | undefined, + version: string | undefined, + source: Exclude, + scope: ArtifactInstallScope | undefined, + ): void { // 1. Register the Package (manifest + lifecycle state). // [ADR-0130 D1/D3] `scope` is the artifact's own package list, passed by // the load path that read the artifact. It is what lets the install gate @@ -9154,19 +9487,8 @@ export class ObjectQL implements IObjectQLEngine { this.logger.debug('Registered manifest-as-app', { app: manifest.name, from: id }); } - // 4b. Register navigation contributions (ADR-0029 D7) — nav items this - // package injects into apps owned by other packages (e.g. a - // capability plugin adding its menu into the `setup` app). Merged - // into the target app's navigation on read by group id + priority. - if (Array.isArray((manifest as any).navigationContributions) && (manifest as any).navigationContributions.length > 0) { - for (const contribution of (manifest as any).navigationContributions) { - this._registry.registerAppNavContribution(contribution, id); - } - this.logger.debug('Registered navigation contributions', { - from: id, - count: (manifest as any).navigationContributions.length, - }); - } + // (Navigation contributions, ADR-0029 D7, register in `registerApp` + // once this body has registered whole.) // 5. Register all other metadata types generically — the SAME seam the // nested-plugin path uses (`registerMetadataCollections`), so the two @@ -9179,10 +9501,9 @@ export class ObjectQL implements IObjectQLEngine { this.logger.debug('Registering seed data datasets', { id, count: seedData.length }); for (const dataset of seedData) { if (dataset.object) { - // [#28534] The collection loop's owner-stamp refusal, for the - // one collection filed outside it. - const ownerStamp = collectionItemOwnerStampRefusal(dataset, 'data', String(dataset.object), id); - if (ownerStamp) throw ownerStamp; + // [#28534] The collection loop's owner-stamp refusal for the + // one collection filed outside it was judged before anything + // registered ({@link packageBodySeedDataRefusal}). const stampBefore = (dataset as { _packageId?: unknown })._packageId; this._registry.registerItem('data', stampPackage(dataset, id, version), 'object' as any, id); noteRegistrationStamp(dataset, stampBefore); @@ -9190,24 +9511,9 @@ export class ObjectQL implements IObjectQLEngine { } } - // 6. Register contributions - if (manifest.contributes?.kinds) { - this.logger.debug('Registering kinds from manifest', { id, kindCount: manifest.contributes.kinds.length }); - for (const kind of manifest.contributes.kinds) { - this._registry.registerKind(kind); - // [commit 10485009a] Name the kind by its declared `id`. `contributes.kinds` - // items are `{ id, description? }` (`manifest.zod.ts`; `globs` was - // retired unread, #11169) and - // `registerKind` keys the item on `id` (`registerItem('kind', kind, 'id')`), - // so `id` is BOTH the only identifying field the schema declares and the - // exact key the item is stored under — a reader of this line can look the - // item straight back up. The previous `kind.name || kind.type` reached for - // two fields NEITHER shape declares, so every conforming manifest logged - // `kind: undefined`. Do not re-add those as a fallback: reading undeclared - // aliases here is the consumer-side tolerance Prime Directive #12 rejects. - this.logger.debug('Registered Kind', { kind: kind.id, from: id }); - } - } + // (Contributed kinds register in `registerApp` once this body has + // registered whole: they belong to no package, so no withdrawal of a + // refused body could take them back.) // 7. Recursively register nested plugins if (Array.isArray(manifest.plugins) && manifest.plugins.length > 0) { @@ -9222,6 +9528,42 @@ export class ObjectQL implements IObjectQLEngine { } } + /** + * [#28697] Take back what a refused first registration of package `id` + * wrote, through the registry's own uninstall ({@link SchemaRegistry.uninstallPackage}): + * the package record and its namespace, its objects and extensions, its apps + * and items, its picklist extensions and catalog claims. Two things that verb + * does not reach are handled here: a nested plugin's own namespace (claimed + * for `id` by `registerObject`), and the boot seed, which an operator + * uninstall forgets with the row but a refused registration keeps, so a later + * install of the id still lands disabled when the persisted state says so. + * + * Nothing to do when the registry refused the package record itself: its + * refusals all precede its first write. A withdrawal that is itself refused + * is logged and the caller's refusal still propagates unchanged. + */ + private withdrawRefusedRegistration(id: string, manifest: any, refusal: unknown): void { + if (this._registry.getPackage(id) === undefined) return; + try { + this._registry.uninstallPackage(id, { keepBootSeed: true }); + if (Array.isArray(manifest.plugins)) { + for (const plugin of manifest.plugins) { + const ns = plugin && typeof plugin === 'object' ? plugin.namespace : undefined; + if (typeof ns === 'string' && ns !== '' && ns !== manifest.namespace) this._registry.unregisterNamespace(ns, id); + } + } + this.invalidateSummaryIndex(); + this.logger.debug('Withdrew a refused package registration', { id }); + } catch (withdrawal) { + this.logger.error( + `Package '${id}' was refused part-way through registering, and what it had registered could not be ` + + 'withdrawn; it stays in this process until restart.', + withdrawal instanceof Error ? withdrawal : undefined, + { id, refusal: String((refusal as Error)?.message ?? refusal), withdrawal: String((withdrawal as Error)?.message ?? withdrawal) }, + ); + } + } + /** * Deep-clone an app definition, resolving objectName references in navigation * items via the registry. Object names are canonical identifiers — no FQN @@ -9317,94 +9659,14 @@ export class ObjectQL implements IObjectQLEngine { this.logger.warn(`Skipping ${pluralToSingular(key)} without a derivable name`, { id: ownerId }); continue; } - // [commit d0ee598e6] The DIVERGENT-container refusal (maintainer ruling - // 2026-09-03, direction 2). This seam used to reconcile a - // container's own `name` to the derived key silently, one line - // below: the author's field discarded with no diagnostic, while - // the OTHER source registrar - // (`MetadataPlugin._parseAndRegisterArtifact` -> - // `assertMetadataRegisterContract`, #7378 row 1) hard-failed the - // whole artifact load on that very same document. One document, - // two SOURCE registrars, opposite outcomes, and which one an - // author got depended on how their package was loaded. #7378 - // row 1 already rules that resolving a name/key disagreement - // silently "in either direction" files the item under a key the - // caller never wrote, so the boot loop converges onto the - // refusal; the artifact door was not moved onto the rewrite. - // - // MEASURED, and it is why only this site changed: the second - // `toRegister` ternary in this method (the assembled - // `viewItems:` channel below) cannot carry a container at all — - // `AssembledViewArtifactSchema` is the view vocabulary MINUS the - // container branch, and every body it admits is - // `isAggregatedViewContainer === false`. Its ternary can only - // MINT a `name` onto an overlay that has none; it can never - // discard an authored one, because `resolveMetadataItemName` - // reads a non-container's own `name` FIRST. - // - // Scope is the ruling's named main risk, so the gate is three - // independent narrowings and each one is load-bearing. The first - // stays here: - // * `key === 'views'` — this is the GENERIC metadata loop; no - // other kind changes behaviour and they all keep the - // reconcile below. - // The other two — the CONTAINER branch only, and `name` present - // AND different — and the message, the envelope and the reason - // the prose is this seam's own live in `viewContainerNameRefusal` - // (`view-container-name-refusal.ts`). - // - // [#20331] Moved there so that `os validate` runs the SAME - // judgment, in the same words, instead of passing a document this - // loop then refuses at boot. The message and the envelope moved - // unchanged. The `!itemName` skip just above is this loop's - // precondition for the check, so the function carries it too: it - // answers `undefined` for a falsy derived key, and a door calling - // it without this loop cannot refuse an entry this loop skips. - // ⛔ Do not re-inline it here, and do not write a second copy of - // it anywhere else: one judge, two doors. - if (key === 'views') { - const refusal = viewContainerNameRefusal(item, sourceLabel, ownerId); - if (refusal) throw refusal; - } - // [ADR-0086 D3] A permission set whose body `packageId` disagrees - // with the package this seam registers it under is refused, in - // the same place and envelope as the container refusal above: - // `registerItem` stamps `ownerId` as the set's `_packageId`, the - // owner every reader reads, so the body's key used to be - // registered over in silence. See - // {@link permissionSetOwnerRegistrationRefusal}. - if (key === 'permissions') { - const refusal = permissionSetOwnerRegistrationRefusal(item, itemName, ownerId); - if (refusal) throw refusal; - } - // [#28534] …and an item of ANY collection whose body already wears - // a `_packageId` other than `ownerId`: `stampPackage` and - // `registerItem` stamp the owner only while the key is unset, so - // the body's own stamp used to win. See - // {@link collectionItemOwnerStampRefusal}. - { - const refusal = collectionItemOwnerStampRefusal(item, pluralToSingular(key), itemName, ownerId); - if (refusal) throw refusal; - } + // [#28697] Every refusal this seam used to raise here — the + // divergent view container, the permission set's owner, an + // item's own `_packageId`, a `views:` entry that is not a + // container — is judged from the body alone, so it is asked + // before anything of the package registers, with the reasons it + // exists: {@link packageBodyCollectionRefusal}. Both callers ask + // it first (`registerApp`, for the body and each nested plugin). const toRegister = item.name === itemName ? item : { ...item, name: itemName }; - // [#5320] The `views:` tighten — containers ONLY, the contract the - // stack schema has always declared (`stack.zod.ts`, - // `z.array(ViewSchema)`). This loop used to register EVERY entry - // as type `view` — the "runtime wider than schema" hole #5320 - // records: a ViewItem or flattened overlay that `defineStack` - // refuses registered here in silence. Judged by the SAME - // classifier the manifest assemblers partition with - // (`isViewContainerShaped`), so a produced manifest cannot carry - // a `views:` entry this seam refuses. Deliberately a SHAPE-CLASS - // judgement, not a full `ViewSchema.parse`: which class an entry - // belongs to is this seam's contract; whether a container's - // internals are well-formed stays the authoring/publish doors' - // job (defineStack, `os validate`, the metadata door). - // The envelope and the words: {@link viewsEntryNotContainerRefusal}, - // which the residual registration throws too. - if (key === 'views' && !isViewContainerShaped(toRegister)) { - throw viewsEntryNotContainerRefusal(itemName, sourceLabel, ownerId); - } const stampBefore = (toRegister as { _packageId?: unknown })._packageId; this._registry.registerItem(pluralToSingular(key), stampPackage(toRegister, ownerId, ownerVersion), 'name' as any, ownerId); noteRegistrationStamp(toRegister, stampBefore); @@ -9440,36 +9702,23 @@ export class ObjectQL implements IObjectQLEngine { if (Array.isArray(assembledItems) && assembledItems.length > 0) { this.logger.debug(`Registering ${ASSEMBLED_VIEW_ITEMS_KEY} from ${sourceLabel}`, { id: ownerId, count: assembledItems.length }); for (const item of assembledItems) { + // [#28697] The entry's parse refusal and its owner stamp were + // judged before anything registered ({@link packageBodyCollectionRefusal}); + // the parse is repeated for the body it registers. const parsed = AssembledViewArtifactSchema.safeParse(item); - if (!parsed.success) { - const itemName = resolveMetadataItemName('views', item) ?? '(unnamed)'; - const err: Error & { code?: string; status?: number; httpStatus?: number } = new Error( - `Invalid \`${ASSEMBLED_VIEW_ITEMS_KEY}:\` entry '${itemName}' from ${sourceLabel} '${ownerId}': ` - + 'the assembled-manifest channel carries non-container view artifacts only — a ViewItem ' - + 'record (`viewKind` + `config`) or a flattened list/form overlay ' - + '(`AssembledViewArtifactSchema`, @objectstack/spec). A view CONTAINER travels in `views:`. ' - + `First issue: ${parsed.error.issues[0]?.message ?? 'no issue detail'}`, - ); - err.code = 'INVALID_METADATA'; - err.status = 422; - err.httpStatus = 422; - throw err; - } + if (!parsed.success) throw assembledViewItemRefusal(item, parsed.error, sourceLabel, ownerId); const body = parsed.data as Record; const itemName = resolveMetadataItemName('views', body); if (!itemName) { this.logger.warn('Skipping viewItems entry without a derivable name', { id: ownerId }); continue; } - // [#28534] The owner-stamp refusal, judged on the PARSED body. No - // {@link noteRegistrationStamp} follows the `registerItem` below, on + // No {@link noteRegistrationStamp} follows the `registerItem` below, on // purpose: what it stamps is `parsed.data`, the parse's own new // object, never the caller's entry, so this site leaves no in-place // stamp on anything a later registration could be handed // (`collection-item-owner-stamp.test.ts` pins the caller's entry // unstamped). - const ownerStamp = collectionItemOwnerStampRefusal(body, 'view', itemName, ownerId); - if (ownerStamp) throw ownerStamp; const toRegister = body.name === itemName ? body : { ...body, name: itemName }; this._registry.registerItem('view', stampPackage(toRegister, ownerId, ownerVersion), 'name' as any, ownerId); } diff --git a/packages/objectql/src/package-refused-registration-withdrawn.test.ts b/packages/objectql/src/package-refused-registration-withdrawn.test.ts new file mode 100644 index 00000000000..2efee40a7e6 --- /dev/null +++ b/packages/objectql/src/package-refused-registration-withdrawn.test.ts @@ -0,0 +1,246 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#28697] A package `registerApp` refuses part-way leaves nothing of itself + * registered, and a refused re-register of an installed id leaves that install + * serving. + * + * ## What this file pins + * + * - every refusal the collection seam and the seed step judge from the body + * alone (an item's own `_packageId`, a divergent view container, a + * permission set's owner, a `views:` entry that is not a container, a + * `viewItems:` entry, a dataset's stamp, the same from a nested plugin) is + * raised with its code and status unchanged, and leaves no package record, + * object, app, navigation contribution, kind, datasource definition or + * nested-plugin namespace behind; on an installed id, the installed version + * still serves, untouched; + * - a refusal only the registry can raise part-way (a field with no `type`, + * an object another package owns, a picklist value already added, the same + * from a nested plugin) on a FIRST registration withdraws what was written, + * leaves the other package's object and list as they were, and keeps the + * boot seed of disabled packages; + * - CONTROLS: a clean body registers everything as before; a body that took + * over a tenant's object is not withdrawn (the withdrawal would drop the + * tenant's layer); an installed id refused part-way by the registry keeps + * today's state, pinned here until it has its own card. + */ + +import { describe, it, expect } from 'vitest'; +import { ObjectQL } from './engine.js'; + +const PKG = 'com.probe.refused'; +const HOST = 'com.probe.host'; + +const object = (name: string, extra: Record = {}) => ({ + name, + label: name, + fields: { title: { type: 'text', label: 'Title' } }, + ...extra, +}); +const app = (name: string) => ({ + name, + label: name, + navigation: [{ id: 'nav_a', type: 'object', label: 'A', objectName: 'probe_a' }], +}); + +/** A body that writes into every registry `registerApp` reaches. */ +const body = (extra: Record = {}) => ({ + id: PKG, + name: PKG, + objects: [object('probe_a')], + apps: [app('probe_app')], + datasources: [{ name: 'probe_ds', driver: 'memory' }], + navigationContributions: [{ app: 'host_app', items: [{ id: 'probe_nav', type: 'url', label: 'Probe', url: '/probe' }] }], + contributes: { kinds: [{ id: 'probe.kind' }] }, + plugins: [{ name: 'probe_inner', namespace: 'probeinner', objects: [object('probe_inner_thing')] }], + ...extra, +}); + +/** What the engine holds of PKG, read through its public verbs. */ +function trace(engine: ObjectQL) { + const registry = engine.registry; + return { + pkg: registry.getPackage(PKG), + objects: registry.getAllObjects().map((o: any) => o.name).filter((n: string) => n.startsWith('probe_')).sort(), + apps: registry.listItems('app').map((a: any) => a.name).sort(), + items: (['page', 'view', 'permission', 'data', 'picklist'] as const) + .flatMap((type) => registry.listItems(type).filter((i: any) => i?._packageId === PKG).map((i: any) => `${type}/${i.name ?? i.object}`)), + nav: registry.getAppNavContributions('host_app').map((c) => c.packageId), + kinds: registry.getAllKinds().map((k) => k.id).filter((id) => id === 'probe.kind'), + datasources: engine.listDatasourceDefs().map((d) => d.name).filter((n) => n === 'probe_ds'), + innerNamespace: registry.getNamespaceOwner('probeinner'), + }; +} + +function expectNothingOf(engine: ObjectQL) { + expect(trace(engine)).toEqual({ + pkg: undefined, objects: [], apps: [], items: [], nav: [], kinds: [], datasources: [], innerNamespace: undefined, + }); +} + +function refusal(fn: () => void): any { + try { + fn(); + } catch (e) { + return e; + } + throw new Error('expected the registration to be refused'); +} + +// ── refusals judged from the body alone ──────────────────────────────────── + +const BODY_ONLY: Array<{ label: string; extra: Record; code: string; status: number }> = [ + { label: 'a page wearing another package\'s `_packageId`', extra: { pages: [{ name: 'probe_page', type: 'app', _packageId: HOST }] }, code: 'VALIDATION_ERROR', status: 400 }, + { label: 'a view container whose own `name` differs from its key', extra: { views: [{ name: 'probe_wrong', object: 'probe_a', list: { type: 'grid', columns: ['title'] } }] }, code: 'VALIDATION_ERROR', status: 400 }, + { label: 'a permission set naming another package', extra: { permissions: [{ name: 'probe_ps', packageId: HOST, objects: {} }] }, code: 'VALIDATION_ERROR', status: 400 }, + { label: 'a `views:` entry that is not a container', extra: { views: [{ name: 'probe_vi', viewKind: 'list', config: {} }] }, code: 'INVALID_METADATA', status: 422 }, + { label: 'a `viewItems:` entry its schema refuses', extra: { viewItems: [{ name: 'probe_a.bad' }] }, code: 'INVALID_METADATA', status: 422 }, + { label: 'a `viewItems:` entry wearing another package\'s `_packageId`', extra: { viewItems: [{ name: 'probe_a.hot', object: 'probe_a', viewKind: 'list', config: { type: 'grid', columns: [{ field: 'title' }] }, _packageId: HOST }] }, code: 'VALIDATION_ERROR', status: 400 }, + { label: 'a seed dataset wearing another package\'s `_packageId`', extra: { data: [{ object: 'probe_a', records: [], _packageId: HOST }] }, code: 'VALIDATION_ERROR', status: 400 }, + { label: 'a nested plugin\'s page wearing another package\'s `_packageId`', extra: { plugins: [{ name: 'probe_inner', namespace: 'probeinner', objects: [object('probe_inner_thing')], pages: [{ name: 'probe_inner_page', type: 'app', _packageId: HOST }] }] }, code: 'VALIDATION_ERROR', status: 400 }, +]; + +describe('[#28697] a body-only refusal leaves nothing of the package behind', () => { + for (const c of BODY_ONLY) { + it(c.label, () => { + const engine = new ObjectQL(); + const err = refusal(() => engine.registerApp(body(c.extra) as any)); + expect(err).toMatchObject({ code: c.code, status: c.status }); + expectNothingOf(engine); + }); + } +}); + +describe('[#28697] a body-only refusal of a re-register leaves the installed version serving', () => { + const v1 = () => ({ + id: PKG, + name: PKG, + version: '1.0.0', + objects: [object('probe_a'), object('probe_old')], + apps: [app('probe_app')], + pages: [{ name: 'probe_v1_page', type: 'app' }], + }); + for (const c of BODY_ONLY) { + it(c.label, () => { + const engine = new ObjectQL(); + engine.registerApp(v1() as any); + const before = trace(engine); + const v2 = body({ + version: '2.0.0', + objects: [object('probe_a', { label: 'v2 label' }), object('probe_new')], + apps: [app('probe_app_v2')], + ...c.extra, + }); + const err = refusal(() => engine.registerApp(v2 as any)); + expect(err).toMatchObject({ code: c.code, status: c.status }); + expect(trace(engine)).toEqual(before); + expect(engine.registry.getPackage(PKG)?.manifest.version).toBe('1.0.0'); + expect((engine.registry.getObject('probe_a') as any)?.label).toBe('probe_a'); + expect(trace(engine).items).toEqual(['page/probe_v1_page']); + }); + } +}); + +// ── refusals only the registry can raise, part-way ───────────────────────── + +describe('[#28697] a registry refusal part-way through a first registration withdraws it', () => { + it('a field with no `type` on a later object', () => { + const engine = new ObjectQL(); + const err = refusal(() => engine.registerApp(body({ objects: [object('probe_a'), { name: 'probe_b', fields: { t: { label: 'No type' } } }] }) as any)); + expect(err).toMatchObject({ code: 'INVALID_METADATA', status: 422 }); + expectNothingOf(engine); + }); + + it('an object another package owns, which keeps it', () => { + const engine = new ObjectQL(); + engine.registerApp({ id: HOST, name: HOST, objects: [object('probe_held', { label: 'Host held' })] } as any); + const err = refusal(() => engine.registerApp(body({ objects: [object('probe_a'), object('probe_held')] }) as any)); + expect(err).toMatchObject({ code: 'OBJECT_OWNERSHIP_CONFLICT', status: 422 }); + expect({ ...trace(engine), objects: trace(engine).objects.filter((n) => n !== 'probe_held') }).toEqual({ + pkg: undefined, objects: [], apps: [], items: [], nav: [], kinds: [], datasources: [], innerNamespace: undefined, + }); + expect(engine.registry.getObjectOwner('probe_held')?.packageId).toBe(HOST); + expect((engine.registry.getObject('probe_held') as any)?.label).toBe('Host held'); + }); + + const INDUSTRY = { name: 'probe_industry', label: 'Industry', options: [{ label: 'Finance', value: 'finance' }] }; + const repeated = { extend: 'probe_industry', options: [{ label: 'Fintech', value: 'finance' }] }; + + it('a picklist value another package\'s list already carries, which keeps its list', () => { + const engine = new ObjectQL(); + engine.registerApp({ id: HOST, name: HOST, picklists: [INDUSTRY] } as any); + const err = refusal(() => engine.registerApp(body({ picklists: [{ name: 'probe_list', label: 'L', options: [{ label: 'X', value: 'x' }] }], picklistExtensions: [repeated] }) as any)); + expect(err).toMatchObject({ code: 'INVALID_METADATA', status: 422 }); + expectNothingOf(engine); + expect(engine.registry.resolvePicklistOptions('probe_list')).toBeUndefined(); + expect(engine.registry.resolvePicklistOptions('probe_industry')).toEqual([{ label: 'Finance', value: 'finance' }]); + }); + + it('the same refusal from a nested plugin, after the parent body registered', () => { + const engine = new ObjectQL(); + engine.registerApp({ id: HOST, name: HOST, picklists: [INDUSTRY] } as any); + const err = refusal(() => engine.registerApp(body({ + plugins: [{ name: 'probe_inner', namespace: 'probeinner', objects: [object('probe_inner_thing')], picklistExtensions: [repeated] }], + }) as any)); + expect(err).toMatchObject({ code: 'INVALID_METADATA', status: 422 }); + expectNothingOf(engine); + }); + + it('keeps the boot seed of disabled packages, so the next install still lands disabled', () => { + const engine = new ObjectQL(); + engine.registry.setInitialDisabledPackageIds([PKG]); + refusal(() => engine.registerApp(body({ objects: [object('probe_a'), { name: 'probe_b', fields: { t: { label: 'No type' } } }] }) as any)); + expectNothingOf(engine); + engine.registerApp(body() as any); + expect(engine.registry.getPackage(PKG)).toMatchObject({ enabled: false, status: 'disabled' }); + }); +}); + +// ── controls ─────────────────────────────────────────────────────────────── + +describe('[#28697] controls', () => { + it('a clean body registers into every registry as before', () => { + const engine = new ObjectQL(); + engine.registerApp(body({ pages: [{ name: 'probe_page', type: 'app' }] }) as any); + const t = trace(engine); + expect(t.pkg).toBeDefined(); + expect(t.objects).toEqual(['probe_a', 'probe_inner_thing']); + expect(t.apps).toEqual(['probe_app']); + expect(t.items).toEqual(['page/probe_page']); + expect(t.nav).toEqual([PKG]); + expect(t.kinds).toEqual(['probe.kind']); + expect(t.datasources).toEqual(['probe_ds']); + expect(t.innerNamespace).toBe(PKG); + }); + + it('a body that took over a tenant\'s object is not withdrawn: the tenant\'s layer keeps serving', () => { + const engine = new ObjectQL(); + engine.registry.registerObject({ ...object('probe_a', { label: 'Tenant label' }), _provenance: 'org' } as any, 'sys_metadata', undefined, 'own'); + const err = refusal(() => engine.registerApp(body({ objects: [object('probe_a'), { name: 'probe_b', fields: { t: { label: 'No type' } } }] }) as any)); + expect(err).toMatchObject({ code: 'INVALID_METADATA', status: 422 }); + expect(engine.registry.getObjectContributors('probe_a').map((c) => `${c.ownership}:${c.packageId}`).sort()) + .toEqual(['overlay:sys_metadata', `own:${PKG}`]); + expect((engine.registry.getObject('probe_a') as any)?.label).toBe('Tenant label'); + expect(engine.registry.getPackage(PKG), 'not withdrawn').toBeDefined(); + }); + + it('an installed id refused part-way by the registry: today\'s state, pinned until its own card', () => { + const engine = new ObjectQL(); + engine.registerApp({ id: HOST, name: HOST, objects: [object('probe_held')] } as any); + engine.registerApp({ id: PKG, name: PKG, version: '1.0.0', objects: [object('probe_a'), object('probe_old')], apps: [app('probe_app')] } as any); + const err = refusal(() => engine.registerApp(body({ + version: '2.0.0', + objects: [object('probe_a', { label: 'v2 label' }), object('probe_held')], + }) as any)); + expect(err).toMatchObject({ code: 'OBJECT_OWNERSHIP_CONFLICT', status: 422 }); + const t = trace(engine); + // Not withdrawn (that would take v1 with it) and not restored. + expect(t.pkg?.manifest.version).toBe('2.0.0'); + expect((engine.registry.getObject('probe_a') as any)?.label).toBe('v2 label'); + expect(t.objects).toEqual(['probe_a', 'probe_held', 'probe_old']); + expect(t.apps).toEqual(['probe_app']); + // What `registerApp` writes only once the body registered whole is not written. + expect({ nav: t.nav, kinds: t.kinds, datasources: t.datasources }).toEqual({ nav: [], kinds: [], datasources: [] }); + }); +}); diff --git a/packages/objectql/src/registry.ts b/packages/objectql/src/registry.ts index bb473270b1f..117441bad1f 100644 --- a/packages/objectql/src/registry.ts +++ b/packages/objectql/src/registry.ts @@ -7298,7 +7298,18 @@ export class SchemaRegistry { } } - uninstallPackage(id: string): boolean { + /** + * Withdraw an installed package: its objects, namespace, items, catalog + * claims and record (each step's reason is on it below). + * + * @param options.keepBootSeed [#28697] Keep `id` in the boot seed of + * disabled packages ({@link setInitialDisabledPackageIds}). The engine's + * withdrawal of a package refused part-way through its first registration + * passes it: that package never existed for an operator, so the persisted + * disable state its next install must honour is not forgotten with the + * row. An operator's uninstall omits it. + */ + uninstallPackage(id: string, options?: { keepBootSeed?: boolean }): boolean { const pkg = this.getPackage(id); if (!pkg) { console.warn(`[Registry] Package not found for uninstall: ${id}`); @@ -7344,7 +7355,7 @@ export class SchemaRegistry { // record removal below rather than at the top: every mutation here is // downstream of `unregisterObjectsByPackage`, the one step that can REFUSE // (ADR-0029), so a refused uninstall still removes nothing at all. - this.initialDisabledPackageIds.delete(id); + if (options?.keepBootSeed !== true) this.initialDisabledPackageIds.delete(id); // Remove package record const collection = this.metadata.get('package'); From da44b701899ebcf95d5aeadb0afdc044598a4954 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 1 Nov 2026 19:01:32 +0000 Subject: [PATCH 2/5] test(objectql,dogfood): pin that a refused package registration leaves nothing behind Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_01FEkQ5kPpaFpvn6zyWoBxLN --- packages/objectql/src/registry-module-mock.ts | 10 ++ ...sed-package-leaves-nothing.dogfood.test.ts | 170 ++++++++++++++++++ 2 files changed, 180 insertions(+) create mode 100644 packages/qa/dogfood/test/install-local-refused-package-leaves-nothing.dogfood.test.ts diff --git a/packages/objectql/src/registry-module-mock.ts b/packages/objectql/src/registry-module-mock.ts index f57ab21ce18..970d66da527 100644 --- a/packages/objectql/src/registry-module-mock.ts +++ b/packages/objectql/src/registry-module-mock.ts @@ -97,6 +97,8 @@ export interface RegistryDoubleInstance { registerItem: any; registerApp: any; installPackage: any; + getPackage: any; + getObjectContributors: any; reset: any; metadata: any; [member: string]: any; @@ -173,6 +175,14 @@ export function createRegistryModuleMock( registerItem: vi.fn(), registerApp: vi.fn(), installPackage: vi.fn(), + // [#28697] `registerApp` asks, before it writes anything, whether a refused + // first registration of the package could be withdrawn: no package record + // under its id, and who already contributes to each object it owns. The + // same lesson as `getAllObjects` above: empty is the truthful body for a + // suite that installs nothing for real, and an omitted member is a hard + // `TypeError` at every `registerApp`. + getPackage: vi.fn(() => undefined), + getObjectContributors: vi.fn(() => []), reset: vi.fn(), metadata: { get: vi.fn(() => new Map()) }, ...instanceOverrides, diff --git a/packages/qa/dogfood/test/install-local-refused-package-leaves-nothing.dogfood.test.ts b/packages/qa/dogfood/test/install-local-refused-package-leaves-nothing.dogfood.test.ts new file mode 100644 index 00000000000..081c75a4207 --- /dev/null +++ b/packages/qa/dogfood/test/install-local-refused-package-leaves-nothing.dogfood.test.ts @@ -0,0 +1,170 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +// +// GOLDEN REGRESSION — a package the install refuses part-way through +// registering leaves nothing of itself in the running system (#28697). +// +// ## What was measured before the fix +// +// Installed into the showcase through `POST /api/v1/marketplace/install-local`, +// an inline manifest whose object and app were clean and whose connector wore +// the showcase's own `_packageId` answered 422 `PLUGIN_REGISTER_FAILED`, with +// no ledger entry; but the package, its object and its app had already +// registered, and stayed readable until restart. +// +// ## What this file pins, on one boot +// +// 1. that install is refused as before, and afterwards neither the package, +// its object nor its app reads; the same id then installs clean, 200; +// 2. an install the registry refuses part-way (its second object is one the +// showcase owns) is refused 422 `PLUGIN_REGISTER_FAILED`, and afterwards +// neither the package nor its first object reads; the showcase's object +// still reads; +// 3. a refused upgrade of an installed package (v2's connector wears the +// showcase's id) leaves v1 serving: its object, its app and the package at +// version 1.0.0 read, and v2's new object does not. +// +// Boots a fixture stack of its own (the install-local plugin mounted), so it +// stays out of `SHARED_SHOWCASE`. + +import { mkdtempSync, rmSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; + +import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import { type VerifyStack } from '@objectstack/verify'; +import { MarketplaceInstallLocalPlugin } from '@objectstack/cloud-connection'; +import { bootShowcase } from './showcase-boot.js'; + +const ROUTE = '/marketplace/install-local'; +/** The showcase's own package id, and an object it owns. */ +const HOST_PACKAGE_ID = 'com.example.showcase'; +const HOST_OBJECT = 'showcase_product'; + +const FRESH_ID = 'com.dogfood.refusedfresh'; +const MIDWAY_ID = 'com.dogfood.refusedmidway'; +const UPGRADE_ID = 'com.dogfood.refusedupgrade'; + +const object = (name: string) => ({ name, label: name, fields: { title: { type: 'text', label: 'Title' } } }); +const app = (name: string, objectName: string) => ({ + name, + label: name, + navigation: [{ id: `nav_${objectName}`, type: 'object', label: objectName, objectName }], +}); +/** A connector naming the showcase as its owner: refused from the body alone. */ +const stampedConnector = (name: string) => ({ + name, + label: 'Claims the host as its owner', + type: 'api', + provider: 'openapi', + providerConfig: { spec: { openapi: '3.0.0', info: { title: 't', version: '1' }, paths: {} } }, + _packageId: HOST_PACKAGE_ID, +}); +const install = (id: string, namespace: string, version: string, extra: Record) => ({ + manifest: { id, namespace, name: `Refused ${namespace}`, version, type: 'app', ...extra }, +}); + +const FRESH = install(FRESH_ID, 'refusedfresh', '1.0.0', { + objects: [object('refusedfresh_thing')], + apps: [app('refusedfresh_app', 'refusedfresh_thing')], + connectors: [stampedConnector('refusedfresh_conn')], +}); +const FRESH_CLEAN = install(FRESH_ID, 'refusedfresh', '1.0.0', { + objects: [object('refusedfresh_thing')], + apps: [app('refusedfresh_app', 'refusedfresh_thing')], +}); +const MIDWAY = install(MIDWAY_ID, 'refusedmidway', '1.0.0', { + objects: [object('refusedmidway_thing'), object(HOST_OBJECT)], +}); +const UPGRADE_V1 = install(UPGRADE_ID, 'refusedupgrade', '1.0.0', { + objects: [object('refusedupgrade_thing')], + apps: [app('refusedupgrade_app', 'refusedupgrade_thing')], +}); +const UPGRADE_V2 = install(UPGRADE_ID, 'refusedupgrade', '2.0.0', { + objects: [object('refusedupgrade_thing'), object('refusedupgrade_new')], + apps: [app('refusedupgrade_app', 'refusedupgrade_thing')], + connectors: [stampedConnector('refusedupgrade_conn')], +}); + +type Answer = { status: number; body: any }; + +async function json(res: Response): Promise { + return { status: res.status, body: await res.json().catch(() => null) }; +} + +describe('dogfood: a package the install refuses part-way leaves nothing of itself registered', () => { + let stack: VerifyStack; + let storageDir: string; + const answers: Record = {}; + + beforeAll(async () => { + storageDir = mkdtempSync(join(tmpdir(), 'dogfood-refused-package-')); + stack = await bootShowcase({ + extraPlugins: [new MarketplaceInstallLocalPlugin({ controlPlaneUrl: 'off', storageDir })], + }); + const token = await stack.signIn(); + const get = async (path: string) => json(await stack.apiAs(token, 'GET', path)); + const post = async (body: unknown) => json(await stack.apiAs(token, 'POST', ROUTE, body)); + + // 1 + answers.fresh = await post(FRESH); + answers.freshPackage = await get(`/packages/${FRESH_ID}`); + answers.freshObject = await get('/meta/object/refusedfresh_thing'); + answers.freshApp = await get('/meta/app/refusedfresh_app'); + answers.freshClean = await post(FRESH_CLEAN); + answers.freshCleanApp = await get('/meta/app/refusedfresh_app'); + + // 2 + answers.midway = await post(MIDWAY); + answers.midwayPackage = await get(`/packages/${MIDWAY_ID}`); + answers.midwayObject = await get('/meta/object/refusedmidway_thing'); + answers.hostObject = await get(`/meta/object/${HOST_OBJECT}`); + + // 3 + answers.v1 = await post(UPGRADE_V1); + answers.v2 = await post(UPGRADE_V2); + answers.upgradePackage = await get(`/packages/${UPGRADE_ID}`); + answers.upgradeObject = await get('/meta/object/refusedupgrade_thing'); + answers.upgradeApp = await get('/meta/app/refusedupgrade_app'); + answers.upgradeNew = await get('/meta/object/refusedupgrade_new'); + }, 300_000); + + afterAll(async () => { + await stack?.stop?.(); + if (storageDir) rmSync(storageDir, { recursive: true, force: true }); + }); + + const shown = (a: Answer) => JSON.stringify(a.body).slice(0, 600); + + it('1: a body-only refusal leaves no package, object or app; the same id then installs clean', () => { + expect(answers.fresh.status, shown(answers.fresh)).toBe(422); + expect(answers.fresh.body?.error?.code).toBe('PLUGIN_REGISTER_FAILED'); + expect(answers.fresh.body?.error?.message).toContain('Drop `_packageId` from connector \'refusedfresh_conn\''); + expect(answers.freshPackage.status, shown(answers.freshPackage)).toBe(404); + expect(answers.freshObject.status, shown(answers.freshObject)).toBe(404); + expect(answers.freshApp.status, shown(answers.freshApp)).toBe(404); + expect(answers.freshClean.status, shown(answers.freshClean)).toBe(200); + expect(answers.freshClean.body?.data?.hotLoaded).toBe(true); + expect(answers.freshCleanApp.status).toBe(200); + }); + + it('2: a refusal the registry raises part-way leaves no package or object; the showcase\'s object still reads', () => { + expect(answers.midway.status, shown(answers.midway)).toBe(422); + expect(answers.midway.body?.error?.code).toBe('PLUGIN_REGISTER_FAILED'); + expect(answers.midway.body?.error?.message).toContain(`already owned by package "${HOST_PACKAGE_ID}"`); + expect(answers.midwayPackage.status, shown(answers.midwayPackage)).toBe(404); + expect(answers.midwayObject.status, shown(answers.midwayObject)).toBe(404); + expect(answers.hostObject.status, shown(answers.hostObject)).toBe(200); + }); + + it('3: a refused upgrade leaves the installed version serving', () => { + expect(answers.v1.status, shown(answers.v1)).toBe(200); + expect(answers.v2.status, shown(answers.v2)).toBe(422); + expect(answers.v2.body?.error?.code).toBe('PLUGIN_REGISTER_FAILED'); + expect(answers.upgradePackage.status, shown(answers.upgradePackage)).toBe(200); + expect(JSON.stringify(answers.upgradePackage.body)).toContain('"version":"1.0.0"'); + expect(JSON.stringify(answers.upgradePackage.body)).not.toContain('"version":"2.0.0"'); + expect(answers.upgradeObject.status).toBe(200); + expect(answers.upgradeApp.status).toBe(200); + expect(answers.upgradeNew.status, shown(answers.upgradeNew)).toBe(404); + }); +}); From bc1584d4930abb1468bd75486ed869cb6eecc6d8 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 1 Nov 2026 19:08:14 +0000 Subject: [PATCH 3/5] chore(changeset): objectql patch for a refused package leaving nothing registered Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_01FEkQ5kPpaFpvn6zyWoBxLN --- .../28697-refused-registration-leaves-nothing.md | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) create mode 100644 .changeset/28697-refused-registration-leaves-nothing.md diff --git a/.changeset/28697-refused-registration-leaves-nothing.md b/.changeset/28697-refused-registration-leaves-nothing.md new file mode 100644 index 00000000000..50121536dec --- /dev/null +++ b/.changeset/28697-refused-registration-leaves-nothing.md @@ -0,0 +1,16 @@ +--- +"@objectstack/objectql": patch +--- + +Clause-②: no +Metadata-protocol: none + +fix(objectql): a package refused while it registers leaves nothing of itself registered + +When `ObjectQL.registerApp` refuses a package, the package is no longer left partly registered. Before, the package record, its objects and its apps could stay in the running system until restart. What is refused, and every refusal's code, status and words, is unchanged. + +- **A first install that is refused** (a fresh package id) leaves no package record, object, extension, app, item, picklist extension, navigation contribution, contributed kind, datasource definition or nested-plugin namespace behind. The same id can then be installed again once the package is fixed. A persisted "disabled" state for that id still applies to its next install. +- **A refused upgrade of an installed package** whose body alone is at fault (for example an item that names another package as its owner) leaves the installed version serving, untouched. +- `SchemaRegistry.uninstallPackage(id, options?)` takes an optional `{ keepBootSeed: true }`. The engine passes it when it withdraws a refused first install. An operator's uninstall omits it and behaves as before. + +A body with more than one defect may now be refused for a different one of them than before, because refusals that depend only on the body are now checked before the others. From 3b764618f0e4ace566bc9dff88625405d919fda0 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 1 Nov 2026 19:20:47 +0000 Subject: [PATCH 4/5] test(objectql,dogfood): tidy the refused-registration test headers Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_01FEkQ5kPpaFpvn6zyWoBxLN --- packages/objectql/src/engine.ts | 9 +++------ .../package-refused-registration-withdrawn.test.ts | 8 ++++---- ...-refused-package-leaves-nothing.dogfood.test.ts | 14 ++++---------- 3 files changed, 11 insertions(+), 20 deletions(-) diff --git a/packages/objectql/src/engine.ts b/packages/objectql/src/engine.ts index 67850752153..1dae8aede59 100644 --- a/packages/objectql/src/engine.ts +++ b/packages/objectql/src/engine.ts @@ -4996,12 +4996,9 @@ export function packageBodyObjectAndAppOwnerStampRefusal( * walking the collections and items in the order the seam registers them, or * `undefined`. * - * The seam used to raise each of these on the item it was about to register, - * after `installPackage`, the objects and the apps had registered: the - * refusal propagated (install-local answered `422 PLUGIN_REGISTER_FAILED`) - * while the package record, objects and apps stayed in the live registry until - * restart, and a refused re-register of an installed id left that install - * half-replaced. Every judge here reads the item, its derived name, the + * Asked first so a body refused by one of them leaves nothing of the package + * registered, and a refused re-register of an installed id leaves that + * install as it was. Every judge here reads the item, its derived name, the * source label and the owning id, and nothing the registry holds, so asking * it first gives the same answer in the same words. ⛔ No second copy of any * judge: each is the one function its own record names. diff --git a/packages/objectql/src/package-refused-registration-withdrawn.test.ts b/packages/objectql/src/package-refused-registration-withdrawn.test.ts index 2efee40a7e6..14b142b876a 100644 --- a/packages/objectql/src/package-refused-registration-withdrawn.test.ts +++ b/packages/objectql/src/package-refused-registration-withdrawn.test.ts @@ -22,8 +22,8 @@ * boot seed of disabled packages; * - CONTROLS: a clean body registers everything as before; a body that took * over a tenant's object is not withdrawn (the withdrawal would drop the - * tenant's layer); an installed id refused part-way by the registry keeps - * today's state, pinned here until it has its own card. + * tenant's layer); nor is an installed id refused part-way by the registry + * (the withdrawal would take the installed version with it). */ import { describe, it, expect } from 'vitest'; @@ -225,7 +225,7 @@ describe('[#28697] controls', () => { expect(engine.registry.getPackage(PKG), 'not withdrawn').toBeDefined(); }); - it('an installed id refused part-way by the registry: today\'s state, pinned until its own card', () => { + it('an installed id refused part-way by the registry is not withdrawn', () => { const engine = new ObjectQL(); engine.registerApp({ id: HOST, name: HOST, objects: [object('probe_held')] } as any); engine.registerApp({ id: PKG, name: PKG, version: '1.0.0', objects: [object('probe_a'), object('probe_old')], apps: [app('probe_app')] } as any); @@ -235,7 +235,7 @@ describe('[#28697] controls', () => { }) as any)); expect(err).toMatchObject({ code: 'OBJECT_OWNERSHIP_CONFLICT', status: 422 }); const t = trace(engine); - // Not withdrawn (that would take v1 with it) and not restored. + // Not withdrawn: the withdrawal would take v1 with it. expect(t.pkg?.manifest.version).toBe('2.0.0'); expect((engine.registry.getObject('probe_a') as any)?.label).toBe('v2 label'); expect(t.objects).toEqual(['probe_a', 'probe_held', 'probe_old']); diff --git a/packages/qa/dogfood/test/install-local-refused-package-leaves-nothing.dogfood.test.ts b/packages/qa/dogfood/test/install-local-refused-package-leaves-nothing.dogfood.test.ts index 081c75a4207..fa0447e9e9e 100644 --- a/packages/qa/dogfood/test/install-local-refused-package-leaves-nothing.dogfood.test.ts +++ b/packages/qa/dogfood/test/install-local-refused-package-leaves-nothing.dogfood.test.ts @@ -3,18 +3,12 @@ // GOLDEN REGRESSION — a package the install refuses part-way through // registering leaves nothing of itself in the running system (#28697). // -// ## What was measured before the fix -// -// Installed into the showcase through `POST /api/v1/marketplace/install-local`, -// an inline manifest whose object and app were clean and whose connector wore -// the showcase's own `_packageId` answered 422 `PLUGIN_REGISTER_FAILED`, with -// no ledger entry; but the package, its object and its app had already -// registered, and stayed readable until restart. -// // ## What this file pins, on one boot // -// 1. that install is refused as before, and afterwards neither the package, -// its object nor its app reads; the same id then installs clean, 200; +// 1. an install whose connector wears the showcase's id is refused as +// before (422 `PLUGIN_REGISTER_FAILED`), and afterwards neither the +// package, its object nor its app reads; the same id then installs +// clean, 200; // 2. an install the registry refuses part-way (its second object is one the // showcase owns) is refused 422 `PLUGIN_REGISTER_FAILED`, and afterwards // neither the package nor its first object reads; the showcase's object From 5203704db40d3659f5233db15c98cb875da2774d Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 1 Nov 2026 20:04:24 +0000 Subject: [PATCH 5/5] chore(changeset): declare the optional uninstallPackage parameter as a widening Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_01FEkQ5kPpaFpvn6zyWoBxLN --- .changeset/28697-refused-registration-leaves-nothing.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/28697-refused-registration-leaves-nothing.md b/.changeset/28697-refused-registration-leaves-nothing.md index 50121536dec..9cbb5ddfd91 100644 --- a/.changeset/28697-refused-registration-leaves-nothing.md +++ b/.changeset/28697-refused-registration-leaves-nothing.md @@ -2,7 +2,7 @@ "@objectstack/objectql": patch --- -Clause-②: no +Clause-②: yes (widening) — `SchemaRegistry.uninstallPackage` gains an optional `options` parameter Metadata-protocol: none fix(objectql): a package refused while it registers leaves nothing of itself registered