diff --git a/.changeset/21276-package-delete-store-refusal.md b/.changeset/21276-package-delete-store-refusal.md new file mode 100644 index 00000000000..058850d9076 --- /dev/null +++ b/.changeset/21276-package-delete-store-refusal.md @@ -0,0 +1,19 @@ +--- +'@objectstack/metadata-protocol': patch +'@objectstack/runtime': patch +'@objectstack/objectql': patch +--- + +fix: when the store refuses an uninstall's `sys_packages` delete, the uninstall now answers the failure and removes nothing else, instead of answering success and coming back after the next restart (#21276) + +Clause-②: no + +**`@objectstack/metadata-protocol`.** `deletePackage` now deletes the package's `sys_packages` row first, before its `sys_metadata` rows, its tables, its registry entry and the rows the uninstall cleanups own. When the `package` service refuses that delete, whether it returns `{ success: false }` or throws, `deletePackage` throws and nothing else is removed. A store fault answers `500`, with `DATABASE_ERROR` from a live SQL driver and `INTERNAL_ERROR` otherwise. A declared 4xx refusal is passed through unchanged. Before this, the refusal was logged as a warning, and `DELETE /api/v1/packages/:id` answered `200` after the package's metadata, tables and grants had been removed. The package then came back after the next restart. + +Before that store delete, `deletePackage` now also asks the registry whether the uninstall would be refused because another package extends an object this package owns (ADR-0029). If so, it throws the registry's own refusal with nothing removed. A registry without the new question is not asked, and the refusal then surfaces at the registry withdrawal, as before. + +**`@objectstack/objectql`.** New: `SchemaRegistry.assertPackageUninstallable(packageId)`. It throws the refusal `unregisterObjectsByPackage` and `uninstallPackage` raise for an object another package extends, with the same message, and it changes nothing. `unregisterObjectsByPackage` now calls it, so there is still one copy of that check. + +**`@objectstack/runtime`.** `DELETE /api/v1/packages/:id` now asks `deletePackage` before it touches anything. It checks that the package exists with a read, and it withdraws the package from the running registry and clears its saved disable record only after `deletePackage` has answered. So when the store refuses, the door answers `500`, the same process keeps serving the package, and a package that was disabled stays disabled after a restart. Before this, the door withdrew the package and cleared its disable record first. A refused delete then left the package missing until a restart, and brought a disabled package back enabled. + +An uninstall refused because another package extends an object this package owns still answers `500` with nothing changed: the stored rows, the registry entry and the disable record all stay as they were, in the same process and after a restart. That refusal is now decided before the store delete, instead of by the door withdrawing the package first. An ordinary uninstall, and a host with no `package` service, are unchanged. diff --git a/packages/metadata-protocol/src/protocol.package-delete-refusal.test.ts b/packages/metadata-protocol/src/protocol.package-delete-refusal.test.ts new file mode 100644 index 00000000000..7ecfc4633c7 --- /dev/null +++ b/packages/metadata-protocol/src/protocol.package-delete-refusal.test.ts @@ -0,0 +1,305 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #21276 — an uninstall the store refused is never answered as success. + * + * ## The defect + * + * `deletePackage` deleted the package's `sys_metadata` rows (and tore down its + * tables), then deleted its `sys_packages` row through the `package` service, + * then withdrew it from the registry and ran the uninstall cleanups. The + * `sys_packages` delete sat inside a `catch` that logged a thrown failure with + * `console.warn`, and a RETURNED `{ success: false }` was never read at all. + * So when the store refused that one delete, the uninstall answered success, + * and `PackageServicePlugin.start()` hydrated the surviving row back into the + * registry on the next boot. Measured at `DELETE /api/v1/packages/:id` on + * SQLite with a trigger refusing the delete: 200, then 404 in the same + * process, then 200 after a restart. + * + * ## The contract pinned here (triage's ruling: refuse before withdrawing) + * + * 1. The `sys_packages` delete is the FIRST durable step, and its refusal is + * the uninstall's refusal — a thrown error with the status and code the + * dispatcher door reads (`resolveThrownHttpError`, the one rule every + * door applies), never a success body. + * 2. Nothing after it ran: the registry still holds the package, and its + * `sys_metadata` rows and the rows an uninstall cleanup owns are all + * still there. ⛔ No undo — there is nothing to undo. + * 3. After a restart — a fresh process over the same store — the package is + * in the state the refusal reported: installed, with its metadata. + * 4. CONTROL: an ordinary uninstall still removes everything, store row + * first, and a restart does not bring it back. + * + * Both failure channels of the service's `delete` are driven + * (`PackageDeleteResult`, `packages/services/service-package/src/index.ts`): + * RETURNED `{ success: false }` (an undeclared driver fault the service + * swallowed) and THROWN (a failure that declared its own answer — what a live + * SQL driver's refused raw statement is, `500 DATABASE_ERROR`). + * + * ## The doubles + * + * `World` is the database: `sysPackages` stands for `sys_packages`, + * `sysMetadata` for the package's `sys_metadata` rows, and `grants` for the + * data-plane rows an uninstall cleanup removes (plugin-security's package + * permission sets). `boot(world)` is one process over it: the registry is + * hydrated from `sysPackages` the way `PackageServicePlugin.start()` does it, + * so a second `boot` over the same world IS a restart. The registry double + * mirrors the real `SchemaRegistry` verbs by name (`installPackage`, + * `getPackage`, `uninstallPackage`, `packages/objectql/src/registry.ts`), and + * the per-item `deleteMetaItem` is replaced by a double that removes the row + * from `sysMetadata` — the same seam `protocol-package-lifecycle.test.ts` + * stubs, since the per-item teardown has its own pins. + */ + +import { describe, it, expect, vi } from 'vitest'; +import { resolveThrownHttpError } from '@objectstack/types'; +import { ObjectStackProtocolImplementation } from './index.js'; + +const PKG = 'com.example.leave'; + +interface MetadataRow { + type: string; + name: string; + state: string; + package_id: string; + organization_id: string | null; +} + +interface World { + sysPackages: Map>; + sysMetadata: MetadataRow[]; + grants: Set; + /** Every step that changes the world or the registry, in the order it ran. */ + steps: string[]; +} + +function makeWorld(): World { + return { + sysPackages: new Map([[PKG, { id: PKG, name: 'Leave', version: '1.0.0', namespace: 'leave' }]]), + sysMetadata: [ + { type: 'object', name: 'leave_request', state: 'active', package_id: PKG, organization_id: null }, + { type: 'view', name: 'leave_request_list', state: 'draft', package_id: PKG, organization_id: null }, + ], + grants: new Set([`${PKG}:leave_manager`]), + steps: [], + }; +} + +type StoreDelete = (world: World, id: string) => Promise; + +/** The delete lands: the row leaves `sys_packages`. */ +const landed: StoreDelete = async (world, id) => { + world.sysPackages.delete(id); + return { success: true }; +}; + +/** The RETURNED channel: the DELETE broke and declared nothing (`PackageDeleteResult`). */ +const returnedRefusal: StoreDelete = async () => ({ success: false }); + +/** The THROWN channel, shaped as a live SQL driver's refused raw statement (`rawStatementFaultError`). */ +const DRIVER_LINE = "DELETE FROM sys_packages WHERE id = 'com.example.leave' - refused by trigger"; +const thrownDatabaseError: StoreDelete = async () => { + throw Object.assign(new Error('The database refused to run a raw statement.'), { + code: 'DATABASE_ERROR', + status: 500, + cause: new Error(DRIVER_LINE), + }); +}; + +/** One process over `world`. A second call over the same world is a restart. */ +function boot(world: World, storeDelete: StoreDelete = landed, opts: { uninstallRefusal?: Error } = {}) { + const rows = new Map; status: string; enabled: boolean }>(); + const registry = { + installPackage(manifest: Record) { + const row = { manifest: { ...manifest }, status: 'installed', enabled: true }; + rows.set(String(manifest.id), row); + return row; + }, + getPackage: (id: string) => rows.get(id), + getAllPackages: () => [...rows.values()], + // The real `SchemaRegistry` verb: asks the uninstall's ADR-0029 refusal and + // mutates nothing. It refuses only when the case says another package + // extends an object this one owns. + assertPackageUninstallable(_id: string) { + if (opts.uninstallRefusal) throw opts.uninstallRefusal; + }, + uninstallPackage(id: string) { + world.steps.push('registry.uninstallPackage'); + return rows.delete(id); + }, + }; + // The boot hydration: every stored row is registered. + for (const manifest of world.sysPackages.values()) registry.installPackage(manifest); + + const engine = { + registry, + // Scalar equality on every `where` key, and the caller's `limit` by + // presence after the filter. A combinator (`$or`, the org-scoped + // uninstall's) is not implemented here, so it is refused, never answered + // as "no rows" — these pins uninstall with `allTenants: true`. + find: async (object: string, query?: { where?: Record; limit?: number }) => { + if (object !== 'sys_metadata') return []; + const where = query?.where ?? {}; + for (const key of Object.keys(where)) { + if (key.startsWith('$')) throw new Error(`find double: '${key}' is not implemented`); + } + const matched = world.sysMetadata.filter((row) => + Object.entries(where).every(([key, value]) => (row as unknown as Record)[key] === value)); + const page = typeof query?.limit === 'number' ? matched.slice(0, query.limit) : matched; + return page.map((row) => ({ ...row })); + }, + }; + const services = new Map([ + ['package', { + publish: async () => ({ success: true }), + delete: async (id: string) => { + world.steps.push('sys_packages.delete'); + return storeDelete(world, id); + }, + }], + ]); + const impl = new ObjectStackProtocolImplementation(engine as never, () => services as never) as any; + vi.spyOn(impl, 'deleteMetaItem').mockImplementation(async (req: any) => { + world.steps.push(`sys_metadata.delete ${req.type}/${req.name}/${req.state}`); + world.sysMetadata = world.sysMetadata.filter( + (r) => !(r.type === req.type && r.name === req.name && r.state === req.state), + ); + return { success: true }; + }); + impl.registerUninstallCleanup('security.package-permissions', async ({ packageId }: { packageId: string }) => { + world.steps.push('cleanup security.package-permissions'); + let removed = 0; + for (const grant of [...world.grants]) { + if (grant.startsWith(`${packageId}:`)) { + world.grants.delete(grant); + removed++; + } + } + return { success: true, removed }; + }); + return { impl, registry }; +} + +/** What the dispatcher door answers for a thrown value (`errorFromThrown` → `resolveThrownHttpError`). */ +const door = (e: unknown) => { + const r = resolveThrownHttpError(e, 500); + return { status: r.status, code: r.code, message: r.message }; +}; + +async function rejectionOf(p: Promise): Promise { + try { + await p; + } catch (e) { + return e; + } + throw new Error('expected the call to reject, and it resolved'); +} + +const snapshot = (world: World) => ({ + sysPackages: [...world.sysPackages.keys()], + sysMetadata: world.sysMetadata.map((r) => `${r.type}/${r.name}/${r.state}`), + grants: [...world.grants], +}); + +const REFUSALS = [ + ['returned { success: false }', 'INTERNAL_ERROR', returnedRefusal], + ['thrown DATABASE_ERROR', 'DATABASE_ERROR', thrownDatabaseError], +] as const; + +describe('#21276 deletePackage — a refused sys_packages delete fails the uninstall and removes nothing', () => { + it.each(REFUSALS)('%s → 500 %s; the registry, the metadata rows and the grants are untouched', async (_label, code, refusal) => { + const world = makeWorld(); + const before = snapshot(world); + const { impl, registry } = boot(world, refusal); + + const err = await rejectionOf(impl.deletePackage({ packageId: PKG, allTenants: true })); + + expect(door(err)).toMatchObject({ status: 500, code }); + // The driver's words stay on `cause` for the operator, never in the caller's sentence. + expect(door(err).message).not.toContain(DRIVER_LINE); + expect((err as { cause?: unknown }).cause).toBeDefined(); + // The store delete was the first and only step: nothing after it ran. + expect(world.steps).toEqual(['sys_packages.delete']); + expect(snapshot(world)).toEqual(before); + expect(registry.getPackage(PKG)).toBeDefined(); + }); + + it('a declared 4xx refusal from the store leaves as the producer answered it, and nothing is removed', async () => { + const refusal = Object.assign(new Error('Refused by the platform.'), { code: 'DESTRUCTIVE_CHANGE', status: 409 }); + const world = makeWorld(); + const before = snapshot(world); + const { impl, registry } = boot(world, async () => { + throw refusal; + }); + + const err = await rejectionOf(impl.deletePackage({ packageId: PKG, allTenants: true })); + + expect(err).toBe(refusal); + expect(door(err)).toMatchObject({ status: 409, code: 'DESTRUCTIVE_CHANGE' }); + expect(world.steps).toEqual(['sys_packages.delete']); + expect(snapshot(world)).toEqual(before); + expect(registry.getPackage(PKG)).toBeDefined(); + }); +}); + +describe('#21276 deletePackage — the registry\'s uninstall refusal is asked BEFORE the store delete', () => { + it('another package extends an object this one owns: the refusal is thrown as is, and the sys_packages row survives', async () => { + const extender = new Error( + 'Cannot uninstall package "com.example.leave": object "leave_request" is extended by com.example.addon. Uninstall extenders first.', + ); + const world = makeWorld(); + const before = snapshot(world); + const { impl, registry } = boot(world, landed, { uninstallRefusal: extender }); + + const err = await rejectionOf(impl.deletePackage({ packageId: PKG, allTenants: true })); + + // The registry's own error, unwrapped: the door answers it as it always did. + expect(err).toBe(extender); + // Asked before the first durable step: not even the store delete ran. + expect(world.steps).toEqual([]); + expect(snapshot(world)).toEqual(before); + expect(registry.getPackage(PKG)).toBeDefined(); + // …and a restart therefore still has the package, whole. + expect(boot(world).registry.getPackage(PKG)).toBeDefined(); + }); +}); + +describe('#21276 the restart — a fresh process over the same store holds the package as the uninstall reported it', () => { + it.each(REFUSALS)('after a %s refusal, the package comes back WITH its metadata and grants', async (_label, _code, refusal) => { + const world = makeWorld(); + await rejectionOf(boot(world, refusal).impl.deletePackage({ packageId: PKG, allTenants: true })); + + const restarted = boot(world); + + // Reported: not uninstalled. So, after a restart: installed, and whole. + expect(restarted.registry.getPackage(PKG)).toBeDefined(); + expect(snapshot(world)).toEqual({ + sysPackages: [PKG], + sysMetadata: ['object/leave_request/active', 'view/leave_request_list/draft'], + grants: [`${PKG}:leave_manager`], + }); + }); + + it('CONTROL — an ordinary uninstall removes the store row first, then the rest, and a restart does not bring it back', async () => { + const world = makeWorld(); + const first = boot(world); + + const res = await first.impl.deletePackage({ packageId: PKG, allTenants: true }); + + expect(res).toMatchObject({ success: true, deletedCount: 2, failedCount: 0 }); + expect(res.cleanups).toEqual([{ name: 'security.package-permissions', success: true, removed: 1 }]); + expect(world.steps).toEqual([ + 'sys_packages.delete', + 'sys_metadata.delete view/leave_request_list/draft', + 'sys_metadata.delete object/leave_request/active', + 'registry.uninstallPackage', + 'cleanup security.package-permissions', + ]); + expect(first.registry.getPackage(PKG)).toBeUndefined(); + + const restarted = boot(world); + + expect(restarted.registry.getPackage(PKG)).toBeUndefined(); + expect(snapshot(world)).toEqual({ sysPackages: [], sysMetadata: [], grants: [] }); + }); +}); diff --git a/packages/metadata-protocol/src/protocol.ts b/packages/metadata-protocol/src/protocol.ts index 237a0530d7a..258e87adcb5 100644 --- a/packages/metadata-protocol/src/protocol.ts +++ b/packages/metadata-protocol/src/protocol.ts @@ -20719,6 +20719,23 @@ export class ObjectStackProtocolImplementation implements * platform storage is never dropped. Drafts are removed before active rows * so each object's table is torn down once. Per-item failures are collected * without aborting the rest. + * + * [#21276] The steps, in order. Nothing durable happens before step 4, so + * a refusal at any of steps 1–4 leaves everything as it was: + * 1. the tenant-scope refusals (`TENANT_SCOPE_REQUIRED`) — pure; + * 2. the `sys_metadata` read — a read; a failure is thrown; + * 3. the registry's uninstall refusal (another package extends an object + * this one owns, ADR-0029), asked through + * `SchemaRegistry.assertPackageUninstallable` — pure; thrown as is; + * 4. the `sys_packages` delete through the `package` service — the FIRST + * durable step; a refusal, returned or thrown, is thrown as this verb's + * failure (see {@link packagePersistFailureError}); + * 5. the per-item `sys_metadata` deletes and table teardown — each refusal + * is collected in `failed[]`; + * 6. the registry withdrawal — step 3 already asked its refusal; anything + * it still throws is logged, and the package leaves at the next + * restart, since its stored row is already gone; + * 7. the uninstall cleanups — each refusal is reported in `cleanups[]`. */ async deletePackage(request: DeletePackageRequest): Promise { // [#7780] A cross-tenant uninstall must be DECLARED, never inferred from @@ -20868,6 +20885,71 @@ export class ObjectStackProtocolImplementation implements throw metadataReadFailureError(e); } + // [#21276] THE REGISTRY'S UNINSTALL REFUSAL, ASKED BEFORE THE STORE + // DELETE. `SchemaRegistry` refuses an uninstall when another package + // `extend`s an object this one owns (ADR-0029), and it decides that + // before it mutates (#7970). Here that refusal used to be met only at + // the registry withdrawal below, after the stored row, the metadata rows + // and the tables were already gone. `assertPackageUninstallable` asks + // the same predicate (the one copy `unregisterObjectsByPackage` itself + // calls) without performing the uninstall, so the refusal is thrown + // here, as is, with nothing removed. The HTTP door answers it through + // its `catch` around this verb: `500`, nothing changed. + // + // The registry is reached the way the withdrawal below reaches it. A + // registry that does not carry the method (an engine double, a host on + // a registry without it) is not asked, and this verb behaves as it did + // before the method existed: the refusal surfaces at the withdrawal. + const packageRegistry = (this.engine as any)?.registry; + if (typeof packageRegistry?.assertPackageUninstallable === 'function') { + packageRegistry.assertPackageUninstallable(request.packageId); + } + + // [#21276] THE STORE DELETE COMES FIRST, and its refusal is this verb's + // refusal. Triage's ruling: refuse before withdrawing, not undo. Every + // step above this one only reads; every step below it — the per-item + // `sys_metadata` deletes and table teardown, the registry withdrawal, + // the uninstall cleanups — runs only after the store has deleted the + // package's row. + // + // #2532's reason for deleting the row at all still holds: + // `PackageServicePlugin.start()` hydrates `sys_packages` back into the + // registry at boot, so a row left behind brings the package back on the + // next restart. This delete used to run AFTER the metadata deletes and + // inside a `catch` that turned both of the service's failure channels + // into a `console.warn` ("sys_packages cleanup skipped"): a returned + // `{ success: false }` was never read, and a thrown failure was only + // logged. So a refused delete answered success over a package whose + // metadata, tables and grants were already gone, and the next boot + // brought it back. Measured at `DELETE /api/v1/packages/:id` on SQLite, + // with a trigger refusing the delete: 200, then 404 in the same process, + // then 200 after a restart. + // + // Both channels (`PackageDeleteResult`, `service-package`) answer + // through {@link packagePersistFailureError}, the error install and + // edit throw for a refused store write (#21243): a declared 4xx leaves + // as the producer answered it; anything else is a 500 that quotes + // nothing, with the original on `cause`. + // + // ⛔ No undo: nothing durable has happened yet, so there is nothing to + // put back. Without a `package` service there is no stored row to + // delete (the install's in-memory-only path), and this step is skipped. + const packageStore = this.getServicesRegistry?.()?.get('package') as + | { delete?: (id: string) => Promise } + | undefined; + if (typeof packageStore?.delete === 'function') { + let refusal: { cause: unknown } | undefined; + try { + const out = await packageStore.delete(request.packageId); + if (typeof out === 'object' && out !== null && (out as { success?: unknown }).success === false) { + refusal = { cause: out }; + } + } catch (cause) { + refusal = { cause }; + } + if (refusal) throw packagePersistFailureError(refusal.cause, request.packageId, 'delete'); + } + const dropStorage = request.keepData !== true; // Delete drafts before active so an object's table is dropped once (on // the active delete), not pre-empted by a draft delete. @@ -20930,28 +21012,18 @@ export class ObjectStackProtocolImplementation implements } } - // #2532 counterpart: also drop the durable `sys_packages` record — - // service-package hydrates that table back into the registry at boot, - // so leaving the row behind would RESURRECT an uninstalled package on - // the next restart. Best-effort, same posture as install persistence. - try { - const pkgSvc = this.getServicesRegistry?.()?.get('package') as - | { delete?: (id: string) => Promise } - | undefined; - if (pkgSvc?.delete) await pkgSvc.delete(request.packageId); - } catch (e) { - console.warn( - `[protocol.deletePackage] sys_packages cleanup skipped for '${request.packageId}': ${(e as Error)?.message}`, - ); - } - // [#2747] Unregister from the in-memory SchemaRegistry too, so the // running kernel stops serving the package without waiting for a - // restart. Best-effort: the HTTP dispatcher already unregisters - // before calling us (second call is a no-op warn), and a package - // with live extenders refuses unregistration — that failure is - // logged, not fatal (the durable row is gone, so the next boot is - // clean either way). + // restart. [#21276] The HTTP door no longer unregisters before calling + // this verb; it withdraws only after this verb has answered, and skips + // that when this step already did it. + // + // [#21276] The registry's own refusal (ADR-0029 extenders) was asked + // before the store delete, through `assertPackageUninstallable`, so it + // does not arrive here. The `catch` stays as a safety net for a + // registry that lacks that method, or a throw nothing asked ahead of + // time: it is logged, not fatal, because the durable row is already + // gone and the next boot is clean either way. try { (this.engine as any)?.registry?.uninstallPackage?.(request.packageId); } catch (e) { @@ -24837,10 +24909,16 @@ async function persistPackageManifest( /** * [#21243] The sentence a caller reads when a package write was refused by the - * store and undone. It quotes nothing but the caller's own package id — the - * driver's words stay on `cause` and in the server log. + * store and undone — or [#21276], for an uninstall, refused by the store before + * anything else was removed. It quotes nothing but the caller's own package + * id — the driver's words stay on `cause` and in the server log. */ -function packagePersistFailureMessage(packageId: string, verb: 'install' | 'update'): string { +function packagePersistFailureMessage(packageId: string, verb: 'install' | 'update' | 'delete'): string { + if (verb === 'delete') { + return `Package '${packageId}' was not uninstalled: the package registry could not delete its stored record, ` + + 'and a package whose record is kept comes back on the next restart, so its metadata, data and grants ' + + 'were left in place. The reason is in the server log.'; + } return verb === 'install' ? `Package '${packageId}' was not installed: the package registry could not store it, so it would ` + 'not survive a restart, and nothing was registered. The reason is in the server log.' @@ -24850,7 +24928,8 @@ function packagePersistFailureMessage(packageId: string, verb: 'install' | 'upda /** * [#21243] The error a package install or edit answers when its - * `sys_packages` write failed. The vocabulary is this file's own, reused: + * `sys_packages` write failed — and [#21276] an uninstall, when its + * `sys_packages` delete did. The vocabulary is this file's own, reused: * * - **A declared 4xx is a refusal** and leaves untouched — the producer's own * status, code and sentence (the #8016 rule every package door applies, @@ -24865,7 +24944,7 @@ function packagePersistFailureMessage(packageId: string, verb: 'install' | 'upda * `500 DATABASE_ERROR`; a returned `driverFault` declares nothing, so the * door derives `INTERNAL_ERROR` from the 500. No code is minted. */ -function packagePersistFailureError(cause: unknown, packageId: string, verb: 'install' | 'update'): Error { +function packagePersistFailureError(cause: unknown, packageId: string, verb: 'install' | 'update' | 'delete'): Error { const { declaredStatus } = resolveThrownHttpError(cause); if (declaredStatus !== undefined && declaredStatus >= 400 && declaredStatus < 500) return cause as Error; const err = new Error(packagePersistFailureMessage(packageId, verb)) as Error & { diff --git a/packages/objectql/src/registry-assert-package-uninstallable.test.ts b/packages/objectql/src/registry-assert-package-uninstallable.test.ts new file mode 100644 index 00000000000..4eccbf2d751 --- /dev/null +++ b/packages/objectql/src/registry-assert-package-uninstallable.test.ts @@ -0,0 +1,82 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #21276 — `SchemaRegistry.assertPackageUninstallable` asks the uninstall's + * ADR-0029 refusal without performing the uninstall. + * + * `deletePackage` (`@objectstack/metadata-protocol`) must decide this refusal + * before it deletes the stored `sys_packages` row, and the only place the + * registry used to decide it was inside `unregisterObjectsByPackage`, which + * mutates when it does not refuse. The refusal pass (#7970) is now this one + * method, and `unregisterObjectsByPackage` calls it: ONE predicate. + * + * Pinned here: + * 1. it refuses with the uninstall's exact sentence, and mutates nothing; + * 2. it answers normally for an uninstallable package, and mutates nothing; + * 3. `unregisterObjectsByPackage` still refuses with that same sentence, by + * calling this method (not a copy of its predicate), and `force` still + * means "do not ask". + */ + +import { describe, it, expect, beforeEach, vi } from 'vitest'; +import { SchemaRegistry } from './registry'; + +const REFUSAL = + 'Cannot uninstall package "com.owner": object "alpha" is extended by com.ext1, com.ext2. Uninstall extenders first.'; + +/** Every contributor of every object the fixture registers, as plain data. */ +function contributorsOf(registry: SchemaRegistry) { + return ['free', 'alpha', 'beta'].map((name) => ({ + name, + resolves: registry.getObject(name) !== undefined, + contributors: registry.getObjectContributors(name).map((c) => `${c.packageId}:${c.ownership}`), + })); +} + +describe('#21276 SchemaRegistry.assertPackageUninstallable', () => { + let registry: SchemaRegistry; + + beforeEach(() => { + registry = new SchemaRegistry({ multiTenant: false }); + // `free` is walked FIRST and is not extended; `alpha` is the first + // refusable object, with two extenders; `beta` is refusable too. + registry.registerObject({ name: 'free', fields: {} } as any, 'com.owner', 'base', 'own'); + registry.registerObject({ name: 'alpha', fields: {} } as any, 'com.owner', 'base', 'own'); + registry.registerObject({ name: 'beta', fields: {} } as any, 'com.owner', 'base', 'own'); + registry.registerObject({ name: 'alpha', fields: {} } as any, 'com.ext1', undefined, 'extend'); + registry.registerObject({ name: 'alpha', fields: {} } as any, 'com.ext2', undefined, 'extend'); + registry.registerObject({ name: 'beta', fields: {} } as any, 'com.ext3', undefined, 'extend'); + }); + + it('refuses with the uninstall\'s exact sentence, and mutates nothing', () => { + const before = contributorsOf(registry); + + expect(() => registry.assertPackageUninstallable('com.owner')).toThrow(REFUSAL); + + expect(contributorsOf(registry)).toEqual(before); + expect(before.every((o) => o.resolves)).toBe(true); + }); + + it('answers normally for a package nothing extends, and mutates nothing', () => { + registry.registerObject({ name: 'free', fields: {} } as any, 'com.ext1', undefined, 'extend'); + const before = contributorsOf(registry); + + expect(() => registry.assertPackageUninstallable('com.ext1')).not.toThrow(); + expect(() => registry.assertPackageUninstallable('com.unknown')).not.toThrow(); + + expect(contributorsOf(registry)).toEqual(before); + }); + + it('unregisterObjectsByPackage refuses with the same sentence BY calling it; force does not ask', () => { + const ask = vi.spyOn(registry, 'assertPackageUninstallable'); + + expect(() => registry.unregisterObjectsByPackage('com.owner')).toThrow(REFUSAL); + expect(ask).toHaveBeenCalledTimes(1); + expect(ask).toHaveBeenCalledWith('com.owner'); + expect(registry.getObject('free')).toBeDefined(); + + ask.mockClear(); + registry.unregisterObjectsByPackage('com.owner', true); + expect(ask).not.toHaveBeenCalled(); + }); +}); diff --git a/packages/objectql/src/registry.ts b/packages/objectql/src/registry.ts index 74cf3cdefca..5b4161ea96b 100644 --- a/packages/objectql/src/registry.ts +++ b/packages/objectql/src/registry.ts @@ -3311,45 +3311,68 @@ export class SchemaRegistry { } } + /** + * [#21276] Would uninstalling `packageId` be refused? Throws the refusal + * {@link unregisterObjectsByPackage} (and so {@link uninstallPackage}) would + * raise, and returns normally otherwise. It reads `objectContributors` and + * mutates nothing, so a caller can ask before a step it cannot take back: + * `deletePackage` (`@objectstack/metadata-protocol`) asks it before it + * deletes the stored `sys_packages` row, so an uninstall this registry would + * refuse is refused with nothing removed. + * + * [#7970] THE REFUSAL PASS — the whole decision, taken before a single + * contribution is removed. This check used to live inline in the mutation + * walk of {@link unregisterObjectsByPackage}, one object at a time, so a + * package owning `account` (free) and `contact` (extended by another + * package) lost `account` on the way to refusing over `contact`: the guard + * that exists to keep a registry whole was itself reached through a + * mutation, and nothing rolled it back. Same predicate and same iteration + * order as the inline check it replaced, so the same object still refuses + * with the same message. + * + * ⛔ ONE predicate: {@link unregisterObjectsByPackage} calls this method + * rather than keeping its own copy, so the question asked ahead and the + * refusal raised by the uninstall cannot disagree. `force` is not a + * parameter here: forcing means not asking, and stays the caller's choice. + * + * @throws Error if the package owns an object another package extends (ADR-0029) + */ + assertPackageUninstallable(packageId: string): void { + for (const [fqn, contributors] of this.objectContributors.entries()) { + const ownedHere = contributors.some( + c => c.packageId === packageId && c.ownership === 'own' + ); + if (!ownedHere) continue; + // Extenders from other packages + const otherExtenders = contributors.filter( + c => c.packageId !== packageId && c.ownership === 'extend' + ); + if (otherExtenders.length > 0) { + throw new Error( + `Cannot uninstall package "${packageId}": object "${fqn}" is extended by ` + + `${otherExtenders.map(c => c.packageId).join(', ')}. Uninstall extenders first.` + ); + } + } + } + /** * Unregister all objects contributed by a package. * * [#7970] **Refuses before it mutates.** If any object this package owns is * extended by another package (ADR-0029), the call throws having removed - * nothing — the refusal is decided across every object first. Callers may - * therefore treat a throw as a no-op, which is what lets - * {@link uninstallPackage} run this verb ahead of its own mutations. + * nothing — the refusal is decided across every object first, by + * {@link assertPackageUninstallable}. Callers may therefore treat a throw as + * a no-op, which is what lets {@link uninstallPackage} run this verb ahead of + * its own mutations. * * @throws Error if trying to uninstall an owner that has extenders */ unregisterObjectsByPackage(packageId: string, force: boolean = false): void { - // [#7970] REFUSAL PASS — the whole decision, taken before a single - // contribution is removed. This check used to live inline in the mutation - // walk below, one object at a time, so a package owning `account` (free) - // and `contact` (extended by another package) lost `account` on the way to - // refusing over `contact`: the guard that exists to keep a registry whole - // was itself reached through a mutation, and nothing rolled it back. Same - // predicate and same iteration order as the inline check it replaces, so - // the same object still refuses with the same message — what changed is - // only that no removal precedes the throw. - if (!force) { - for (const [fqn, contributors] of this.objectContributors.entries()) { - const ownedHere = contributors.some( - c => c.packageId === packageId && c.ownership === 'own' - ); - if (!ownedHere) continue; - // Extenders from other packages - const otherExtenders = contributors.filter( - c => c.packageId !== packageId && c.ownership === 'extend' - ); - if (otherExtenders.length > 0) { - throw new Error( - `Cannot uninstall package "${packageId}": object "${fqn}" is extended by ` + - `${otherExtenders.map(c => c.packageId).join(', ')}. Uninstall extenders first.` - ); - } - } - } + // [#7970] REFUSAL PASS — see {@link assertPackageUninstallable}, which holds + // the one copy of the predicate ([#21276] extracted so it can be asked + // without uninstalling). + if (!force) this.assertPackageUninstallable(packageId); // MUTATION PASS — carries no refusal of its own; the pass above already // proved every removal below is allowed. Keep it that way: a second copy of diff --git a/packages/runtime/src/domains/packages-uninstall-envelope.test.ts b/packages/runtime/src/domains/packages-uninstall-envelope.test.ts index 6d58be55a2c..dcfbeb33bc9 100644 --- a/packages/runtime/src/domains/packages-uninstall-envelope.test.ts +++ b/packages/runtime/src/domains/packages-uninstall-envelope.test.ts @@ -76,10 +76,20 @@ const authed = (caps: string[] = ['manage_metadata']): any => ({ }); function make(deletePackageResult: any, opts: { registryRemoved?: boolean } = {}) { + // [#21276] The door reads existence with `getPackage` before it asks + // `deletePackage`, and withdraws only afterwards, so both verbs read one + // `registered` flag: `registryRemoved: false` is a package the registry + // does not hold, exactly as `SchemaRegistry` would answer it. + let registered = opts.registryRemoved ?? true; + const pkg = { id: 'com.example.pkg-a', manifest: { id: 'com.example.pkg-a', name: 'A' } }; const registry = { getAllPackages: vi.fn().mockReturnValue([]), - getPackage: vi.fn().mockReturnValue({ id: 'com.example.pkg-a', manifest: { id: 'com.example.pkg-a', name: 'A' } }), - uninstallPackage: vi.fn().mockReturnValue(opts.registryRemoved ?? true), + getPackage: vi.fn(() => (registered ? pkg : undefined)), + uninstallPackage: vi.fn(() => { + const removed = registered; + registered = false; + return removed; + }), }; const protocol = { deletePackage: vi.fn().mockResolvedValue(deletePackageResult), diff --git a/packages/runtime/src/domains/packages.ts b/packages/runtime/src/domains/packages.ts index 6884f2c35da..76191aab5c3 100644 --- a/packages/runtime/src/domains/packages.ts +++ b/packages/runtime/src/domains/packages.ts @@ -450,6 +450,11 @@ function requireWritablePackage( * asks, so a mirror that exempted anyone would disagree with it. Returns a * refusal result to short-circuit on, or `null` to proceed. Callers MUST run * it before `uninstallPackage`, and only when `deletePackage` will run. + * + * [#21276] The ordering above is the one this refusal was written against. + * Since then the door withdraws nothing before `deletePackage` answers: the + * registry withdrawal and the disable-record clear both follow it, so a + * refusal from the store leaves the running process untouched as well. */ function requireUninstallOrganizationScope( deps: DomainHandlerDeps, @@ -2031,35 +2036,25 @@ export async function handlePackagesRequest(deps: DomainHandlerDeps, path: strin // The protocol keeps its own refusal as the second line. const protocol = await resolveProtocol(deps, _context); const organizationId = await deps.resolveActiveOrganizationId(_context); - if (protocol && typeof protocol.deletePackage === 'function') { + const persists = Boolean(protocol && typeof protocol.deletePackage === 'function'); + if (persists) { const unscoped = requireUninstallOrganizationScope(deps, id, organizationId); if (unscoped) return unscoped; } - const registryRemoved = registry.uninstallPackage(id); - // ⭐ [#18877 ruling item 3] A package that no longer exists has no - // lifecycle state — so the DURABLE disable record goes with the row, - // and the next install of this id is a FRESH install that lands at - // the declared default. The registry half of the same sentence is - // inside `uninstallPackage`, which forgets the id from the boot seed - // set; this is the half that outlives the process. - // - // Without it the record was immortal: `DELETE` removed the row and - // left the id listed on disk, the next boot seeded it back, and a - // reinstalled package came up disabled with nothing anywhere saying - // why — a disable the operator could no longer even see to undo, - // since the package it named was gone. Written only when the - // registry really removed the row, so a 404 changes no state. + // [#21276] Existence is READ here, never acted on. This line used to + // be `registry.uninstallPackage(id)`, and the disable-record clear + // below sat right after it — both BEFORE `deletePackage`. So when the + // store then refused the `sys_packages` delete, the door answered the + // failure while the running process had already dropped the package + // (`GET` 404 until a restart brought it back) and the disable record + // was already gone (a disabled package came back enabled). Measured + // at this door on SQLite with a trigger refusing the delete. // - // Same best-effort try/catch as the install and PATCH arms above: - // the uninstall itself already happened, so a state-file failure - // must not turn it into a 500. - if (registryRemoved) { - try { - setPackageDisabled(_context?.environmentId, id, false); - } catch (err) { - console.warn('[handlePackages] failed to clear persisted disable state on delete', { id, error: (err as Error)?.message }); - } - } + // Triage's ruling for #21276 — refuse before withdrawing, not undo: + // `deletePackage` deletes the stored row FIRST and refuses before it + // removes anything else, so the door asks it first and touches the + // running registry and the disable record only once it has answered. + const existed = registry.getPackage(id) !== undefined; // Persisted removal (AI/runtime packages live in sys_metadata, not // just the in-memory registry — the registry uninstall alone would @@ -2102,10 +2097,74 @@ export async function handlePackagesRequest(deps: DomainHandlerDeps, path: strin ...(keepData ? { keepData: true } : {}), }); } catch (e: any) { + // [#21276] Nothing was touched on this path: the registry + // still holds the package and its disable record is intact. return { handled: true, response: deps.errorFromThrown(e, 500) }; } } + // [#21276] Only now — the persisted delete has answered, or this host + // has none — is the package withdrawn from the running registry. + // `deletePackage` withdraws it itself on the same registry (its own + // step after the stored rows), so this is usually a no-op and is + // skipped when the package is already gone. A host with no + // persisted half keeps exactly its old behaviour: the withdrawal is + // the uninstall, and its refusal (another package extends an object + // this one owns, ADR-0029) is the request's refusal. + // + // With a persisted half, that refusal does not arrive here: + // `deletePackage` asks it (`SchemaRegistry.assertPackageUninstallable`) + // before its store delete and throws it with nothing removed, which + // the `catch` above answers — `500`, nothing changed. This `try` + // stays as a safety net for a registry without that method, or a + // throw nothing asked ahead of time. The stored rows are already + // gone by then, so it is reported as `registryRemoved: false` + // rather than as a failure the store does not bear out, and the + // package leaves the running process at the next restart. + if (registry.getPackage(id) !== undefined) { + if (persists) { + try { + registry.uninstallPackage(id); + } catch (err) { + console.warn( + `[handlePackages] '${id}' was deleted from storage but the running registry refused to ` + + `withdraw it, so this process keeps serving it until it restarts: ${(err as Error)?.message}`, + ); + } + } else { + registry.uninstallPackage(id); + } + } + const registryRemoved = existed && registry.getPackage(id) === undefined; + + // ⭐ [#18877 ruling item 3] A package that no longer exists has no + // lifecycle state — so the DURABLE disable record goes with the row, + // and the next install of this id is a FRESH install that lands at + // the declared default. The registry half of the same sentence is + // inside `uninstallPackage`, which forgets the id from the boot seed + // set; this is the half that outlives the process. + // + // Without it the record was immortal: `DELETE` removed the row and + // left the id listed on disk, the next boot seeded it back, and a + // reinstalled package came up disabled with nothing anywhere saying + // why — a disable the operator could no longer even see to undo, + // since the package it named was gone. Written only for a package + // this request found, so a 404 changes no state; and [#21276] only + // once the persisted delete has answered — with a persisted half, + // whenever its stored row is gone, even if the running registry + // refused the withdrawal above. + // + // Same best-effort try/catch as the install and PATCH arms above: + // the uninstall itself already happened, so a state-file failure + // must not turn it into a 500. + if (persists ? existed : registryRemoved) { + try { + setPackageDisabled(_context?.environmentId, id, false); + } catch (err) { + console.warn('[handlePackages] failed to clear persisted disable state on delete', { id, error: (err as Error)?.message }); + } + } + const deletedCount = persisted?.deletedCount ?? 0; const failedCount = persisted?.failedCount ?? 0; @@ -2149,7 +2208,10 @@ export async function handlePackagesRequest(deps: DomainHandlerDeps, path: strin ), }; } - if (!registryRemoved && deletedCount === 0) { + // [#21276] `existed`, not `registryRemoved`: a package this request + // found whose withdrawal the registry refused after its stored row + // was deleted is not "not found". + if (!existed && deletedCount === 0) { return { handled: true, response: deps.error(`Package '${id}' not found`, 404) }; } // [#16781] `packageId` is REQUIRED by diff --git a/packages/runtime/src/package-uninstall-store-refusal.integration.test.ts b/packages/runtime/src/package-uninstall-store-refusal.integration.test.ts new file mode 100644 index 00000000000..d17c8737893 --- /dev/null +++ b/packages/runtime/src/package-uninstall-store-refusal.integration.test.ts @@ -0,0 +1,281 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #21276 — `DELETE /api/v1/packages/:id` when the store refuses the + * `sys_packages` delete. + * + * ## The defect, as measured at this door + * + * On SQLite, with a trigger refusing `DELETE` on `sys_packages`, the door + * answered `200` with `success: true`, the same process then answered `404`, and + * after a restart the package was back. With `deletePackage` refusing first, + * the door answered `500` but the same process still answered `404`. A + * package disabled beforehand also came back ENABLED after the restart. The + * door had already run `registry.uninstallPackage(id)` and + * `setPackageDisabled(environmentId, id, false)` before it asked the store. + * + * ## The contract pinned here (triage's ruling: refuse before withdrawing) + * + * 1. A refused store delete answers the failure, and the same process still + * serves the package, still disabled, with its metadata. + * 2. After a restart, the package is still there and still disabled. + * 3. CONTROL: an ordinary delete removes it — `404` in the same process, `404` + * after a restart (no resurrection) — and clears its disable record. + * 4. The registry's own uninstall refusal — another package extends an object + * this one owns (ADR-0029) — is decided BEFORE the store delete + * (`SchemaRegistry.assertPackageUninstallable`, asked by `deletePackage`): + * the door answers `500` and nothing changes — the stored rows, the + * registry entry and the disable record are all intact, in the same + * process and after a restart. That is the envelope the door gave when it + * withdrew the package itself first. + * + * ## The composition — the shipped pieces, booted twice over one database file + * + * A REAL `ObjectQL` over a REAL `SqlDriver` (better-sqlite3, on disk); the REAL + * `PackageServicePlugin`, whose `start()` creates `sys_packages`, registers the + * `package` service and hydrates the stored rows into the registry; the REAL + * `ObjectStackProtocolImplementation`; and the REAL `HttpDispatcher`. Before + * the plugin starts, each boot plants the persisted disabled ids the way + * `AppPlugin.seedPersistedDisabledPackages` does + * (`loadDisabledPackageIds` → `setInitialDisabledPackageIds`). A second `boot` + * over the same directory, after the first engine is destroyed, IS the + * restart. `OS_HOME` is a temp directory, so the disable record is a real file + * that only this file writes. + */ + +import { describe, it, expect, beforeAll, afterAll, afterEach, beforeEach, vi } from 'vitest'; +import { mkdtempSync, rmSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { ObjectQL } from '@objectstack/objectql'; +import { SqlDriver } from '@objectstack/driver-sql'; +import { ObjectStackProtocolImplementation } from '@objectstack/metadata-protocol'; +import { + SysMetadataObject, + SysMetadataHistoryObject, + SysMetadataAuditObject, +} from '@objectstack/metadata-core'; +import { PackageServicePlugin } from '@objectstack/service-package'; +import { HttpDispatcher } from './http-dispatcher.js'; +import { loadDisabledPackageIds } from './package-state-store.js'; + +const PKG = 'com.acme.leave'; +const OTHER_PKG = 'com.acme.addon'; +const PLATFORM_PKG = 'com.objectstack.platform'; +const ENV = 'platform'; +const ORG = 'org_acme'; + +/** A signed-in admin acting in an organization — the caller the dev-server measurement used. */ +const CALLER: any = { + request: {}, + environmentId: ENV, + executionContext: { + userId: 'u_admin', + isSystem: false, + systemPermissions: ['manage_metadata', 'studio.access'], + tenantId: ORG, + }, +}; + +const quiet = { debug() {}, info() {}, warn() {}, error() {} }; + +let cleanup: Array<() => void | Promise> = []; +afterEach(async () => { + for (const c of cleanup.reverse()) await c(); + cleanup = []; +}); + +const envSnapshot = { OS_HOME: process.env.OS_HOME }; +let home: string; +beforeAll(() => { + home = mkdtempSync(join(tmpdir(), 'os-21276-home-')); + process.env.OS_HOME = home; +}); +afterAll(() => { + if (envSnapshot.OS_HOME === undefined) delete process.env.OS_HOME; + else process.env.OS_HOME = envSnapshot.OS_HOME; + rmSync(home, { recursive: true, force: true }); +}); + +let warnSpy: ReturnType; +beforeEach(() => { warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}); }); +afterEach(() => { warnSpy.mockRestore(); }); + +/** One process over the database in `dir`. */ +async function boot(dir: string) { + const driver = new SqlDriver({ + client: 'better-sqlite3', + connection: { filename: join(dir, 'data.sqlite') }, + useNullAsDefault: true, + }); + const objects = [SysMetadataObject, SysMetadataHistoryObject, SysMetadataAuditObject] as any[]; + await driver.initObjects(objects); + + const engine = new ObjectQL(); + engine.registerDriver(driver as any, true); + await engine.init(); + for (const o of objects) engine.registry.registerObject(o, PLATFORM_PKG); + // The boot seed, planted before any package row is installed. + engine.registry.setInitialDisabledPackageIds(loadDisabledPackageIds(ENV)); + + const services = new Map([['objectql', engine]]); + const ctx: any = { + logger: quiet, + getService: (name: string) => services.get(name), + registerService: (name: string, service: unknown) => { services.set(name, service); }, + }; + const plugin = new PackageServicePlugin(); + await plugin.init(ctx); + await plugin.start(ctx); + + const protocol = new ObjectStackProtocolImplementation(engine as any, () => services as any, undefined, 'package-author'); + services.set('protocol', protocol); + const get = (name: string) => services.get(name) ?? null; + const dispatcher = new HttpDispatcher({ context: { getService: get }, getService: get, getServiceAsync: async (n: string) => get(n) } as any); + + let destroyed = false; + const destroy = async () => { + if (destroyed) return; + destroyed = true; + await engine.destroy(); + }; + cleanup.push(destroy); + + return { + engine, + protocol, + destroy, + async call(method: string, path: string) { + const res = await dispatcher.handlePackages(path, method, undefined, {}, CALLER); + const body = JSON.parse(JSON.stringify(res.response?.body ?? null)); + return { status: res.response?.status ?? 0, code: body?.error?.code as string | undefined, body }; + }, + async metadataRows() { + const rows = (await engine.find('sys_metadata', { where: { package_id: PKG } })) as any[]; + return rows.map((r) => `${r.type}/${r.name}`).sort(); + }, + }; +} + +type Process = Awaited>; + +/** Install the package, give it one stored view, and disable it through the door. */ +async function seed(p: Process) { + await p.protocol.installPackage({ manifest: { id: PKG, name: 'Leave', version: '1.0.0', type: 'app' } } as any); + await (p.protocol as any).saveMetaItem({ + type: 'view', + name: 'leave_list', + item: { + name: 'leave_list', + label: 'Leave', + type: 'grid', + object: 'anything', + viewKind: 'list', + data: { provider: 'object', object: 'anything' }, + columns: ['id'], + }, + packageId: PKG, + }); + const disabled = await p.call('PATCH', `/${PKG}/disable`); + expect(disabled.status, 'precondition: the package is disabled through the door').toBe(200); + expect(loadDisabledPackageIds(ENV).has(PKG), 'precondition: the disable record is on disk').toBe(true); +} + +/** The forced store refusal: a trigger refusing `DELETE` on `sys_packages` for this package. */ +async function refuseStoreDelete(p: Process) { + await (p.engine as any).execute({ + sql: `CREATE TRIGGER refuse_${PKG.replace(/\./g, '_')}_delete BEFORE DELETE ON sys_packages ` + + `WHEN OLD.id = '${PKG}' BEGIN SELECT RAISE(ABORT, 'refused by the test trigger'); END`, + }); +} + +function newDir() { + const dir = mkdtempSync(join(tmpdir(), 'os-21276-db-')); + cleanup.push(() => rmSync(dir, { recursive: true, force: true })); + return dir; +} + +describe('#21276 DELETE /packages/:id — a refused sys_packages delete changes nothing', () => { + it('answers 500 DATABASE_ERROR, and the same process still serves the package, disabled, with its metadata', async () => { + const p = await boot(newDir()); + await seed(p); + await refuseStoreDelete(p); + + const answer = await p.call('DELETE', `/${PKG}`); + + expect({ status: answer.status, code: answer.code }).toEqual({ status: 500, code: 'DATABASE_ERROR' }); + const detail = await p.call('GET', `/${PKG}`); + expect(detail.status).toBe(200); + expect(detail.body?.data).toMatchObject({ enabled: false, status: 'disabled' }); + expect(await p.metadataRows()).toEqual(['view/leave_list']); + }); + + it('after a restart the package is still installed, still DISABLED, and still has its metadata', async () => { + const dir = newDir(); + const first = await boot(dir); + await seed(first); + await refuseStoreDelete(first); + expect((await first.call('DELETE', `/${PKG}`)).status).toBe(500); + await first.destroy(); + + const restarted = await boot(dir); + + const detail = await restarted.call('GET', `/${PKG}`); + expect(detail.status).toBe(200); + expect(detail.body?.data).toMatchObject({ enabled: false, status: 'disabled' }); + expect(loadDisabledPackageIds(ENV).has(PKG)).toBe(true); + expect(await restarted.metadataRows()).toEqual(['view/leave_list']); + }); +}); + +describe('#21276 CONTROL — an ordinary delete removes the package, and a restart does not bring it back', () => { + it('200, then 404 in the same process, 404 after a restart, its metadata gone and its disable record cleared', async () => { + const dir = newDir(); + const first = await boot(dir); + await seed(first); + + const answer = await first.call('DELETE', `/${PKG}`); + + expect(answer.status).toBe(200); + expect(answer.body?.data).toMatchObject({ packageId: PKG, success: true, registryRemoved: true }); + expect((await first.call('GET', `/${PKG}`)).status).toBe(404); + expect(await first.metadataRows()).toEqual([]); + expect(loadDisabledPackageIds(ENV).has(PKG)).toBe(false); + await first.destroy(); + + const restarted = await boot(dir); + + expect((await restarted.call('GET', `/${PKG}`)).status).toBe(404); + }); +}); + +describe('#21276 the registry\'s uninstall refusal (an ADR-0029 extender) is decided before the store delete', () => { + it('500, and nothing changes: stored rows, registry entry and disable record intact, in the same process and after a restart', async () => { + const dir = newDir(); + const first = await boot(dir); + await seed(first); + // Another package extends an object this one owns, so the registry refuses + // the uninstall. Registered in memory only: nothing about it is stored. + first.engine.registry.registerObject({ name: 'leave_request', fields: { title: { type: 'text' } } } as any, PKG, 'leave', 'own'); + const fqn = first.engine.registry.getAllObjects(PKG).map((o: any) => o.name).find((n: string) => n.endsWith('leave_request')); + first.engine.registry.registerObject({ name: fqn, fields: { note: { type: 'text' } } } as any, OTHER_PKG, undefined, 'extend'); + + const answer = await first.call('DELETE', `/${PKG}`); + + expect({ status: answer.status, code: answer.code }).toEqual({ status: 500, code: 'INTERNAL_ERROR' }); + const detail = await first.call('GET', `/${PKG}`); + expect(detail.status).toBe(200); + expect(detail.body?.data).toMatchObject({ enabled: false, status: 'disabled' }); + expect(first.engine.registry.getObject(fqn)).toBeDefined(); + expect(await first.metadataRows()).toEqual(['view/leave_list']); + expect(loadDisabledPackageIds(ENV).has(PKG)).toBe(true); + await first.destroy(); + + const restarted = await boot(dir); + + const after = await restarted.call('GET', `/${PKG}`); + expect(after.status).toBe(200); + expect(after.body?.data).toMatchObject({ enabled: false, status: 'disabled' }); + expect(await restarted.metadataRows()).toEqual(['view/leave_list']); + }); +});