Skip to content

Commit 9688bde

Browse files
committed
fix(objectql, metadata-protocol): deletePackage asks the registry's uninstall refusal before the sys_packages delete
The ADR-0029 extender refusal was decided only by performing the uninstall, which now runs after the store delete. Its refusal pass becomes SchemaRegistry.assertPackageUninstallable, which unregisterObjectsByPackage itself calls (one predicate), and deletePackage asks it before its first durable step, so the refusal is thrown with nothing removed. The door's late-withdrawal comment no longer says the refusal arrives there. Claude-Session: https://claude.ai/code/session_01DiCSbmJrkzNhuEAier4VoJ Co-authored-by: Claude <noreply@anthropic.com>
1 parent 2db4582 commit 9688bde

3 files changed

Lines changed: 103 additions & 55 deletions

File tree

‎packages/metadata-protocol/src/protocol.ts‎

Lines changed: 41 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -20720,19 +20720,22 @@ export class ObjectStackProtocolImplementation implements
2072020720
* so each object's table is torn down once. Per-item failures are collected
2072120721
* without aborting the rest.
2072220722
*
20723-
* [#21276] The steps, in order. Nothing durable happens before step 3, so
20724-
* a refusal at any of steps 1–3 leaves everything as it was:
20723+
* [#21276] The steps, in order. Nothing durable happens before step 4, so
20724+
* a refusal at any of steps 1–4 leaves everything as it was:
2072520725
* 1. the tenant-scope refusals (`TENANT_SCOPE_REQUIRED`) — pure;
2072620726
* 2. the `sys_metadata` read — a read; a failure is thrown;
20727-
* 3. the `sys_packages` delete through the `package` service — the FIRST
20727+
* 3. the registry's uninstall refusal (another package extends an object
20728+
* this one owns, ADR-0029), asked through
20729+
* `SchemaRegistry.assertPackageUninstallable` — pure; thrown as is;
20730+
* 4. the `sys_packages` delete through the `package` service — the FIRST
2072820731
* durable step; a refusal, returned or thrown, is thrown as this verb's
2072920732
* failure (see {@link packagePersistFailureError});
20730-
* 4. the per-item `sys_metadata` deletes and table teardown — each refusal
20733+
* 5. the per-item `sys_metadata` deletes and table teardown — each refusal
2073120734
* is collected in `failed[]`;
20732-
* 5. the registry withdrawal — a refusal (another package extends an
20733-
* object this one owns, ADR-0029) is logged, and the package leaves at
20734-
* the next restart, since its stored row is already gone;
20735-
* 6. the uninstall cleanups — each refusal is reported in `cleanups[]`.
20735+
* 6. the registry withdrawal — step 3 already asked its refusal; anything
20736+
* it still throws is logged, and the package leaves at the next
20737+
* restart, since its stored row is already gone;
20738+
* 7. the uninstall cleanups — each refusal is reported in `cleanups[]`.
2073620739
*/
2073720740
async deletePackage(request: DeletePackageRequest): Promise<DeletePackageResponse> {
2073820741
// [#7780] A cross-tenant uninstall must be DECLARED, never inferred from
@@ -20882,6 +20885,26 @@ export class ObjectStackProtocolImplementation implements
2088220885
throw metadataReadFailureError(e);
2088320886
}
2088420887

20888+
// [#21276] THE REGISTRY'S UNINSTALL REFUSAL, ASKED BEFORE THE STORE
20889+
// DELETE. `SchemaRegistry` refuses an uninstall when another package
20890+
// `extend`s an object this one owns (ADR-0029), and it decides that
20891+
// before it mutates (#7970). Here that refusal used to be met only at
20892+
// the registry withdrawal below, after the stored row, the metadata rows
20893+
// and the tables were already gone. `assertPackageUninstallable` asks
20894+
// the same predicate (the one copy `unregisterObjectsByPackage` itself
20895+
// calls) without performing the uninstall, so the refusal is thrown
20896+
// here, as is, with nothing removed. The HTTP door answers it through
20897+
// its `catch` around this verb: `500`, nothing changed.
20898+
//
20899+
// The registry is reached the way the withdrawal below reaches it. A
20900+
// registry that does not carry the method (an engine double, a host on
20901+
// a registry without it) is not asked, and this verb behaves as it did
20902+
// before the method existed: the refusal surfaces at the withdrawal.
20903+
const packageRegistry = (this.engine as any)?.registry;
20904+
if (typeof packageRegistry?.assertPackageUninstallable === 'function') {
20905+
packageRegistry.assertPackageUninstallable(request.packageId);
20906+
}
20907+
2088520908
// [#21276] THE STORE DELETE COMES FIRST, and its refusal is this verb's
2088620909
// refusal. Triage's ruling: refuse before withdrawing, not undo. Every
2088720910
// step above this one only reads; every step below it — the per-item
@@ -20991,18 +21014,16 @@ export class ObjectStackProtocolImplementation implements
2099121014

2099221015
// [#2747] Unregister from the in-memory SchemaRegistry too, so the
2099321016
// running kernel stops serving the package without waiting for a
20994-
// restart. Best-effort: the HTTP dispatcher already unregisters
20995-
// before calling us (second call is a no-op warn), and a package
20996-
// with live extenders refuses unregistration — that failure is
20997-
// logged, not fatal (the durable row is gone, so the next boot is
20998-
// clean either way).
20999-
//
21000-
// [#21276] This refusal can still come AFTER the store delete above,
21001-
// and it stays here. `SchemaRegistry` has no verb that answers "would
21002-
// this uninstall be refused?" without performing it; a copy of its
21003-
// extender predicate here would be a second place that must agree with
21004-
// the first; and performing the uninstall first would withdraw the
21005-
// package before the store decides.
21017+
// restart. [#21276] The HTTP door no longer unregisters before calling
21018+
// this verb; it withdraws only after this verb has answered, and skips
21019+
// that when this step already did it.
21020+
//
21021+
// [#21276] The registry's own refusal (ADR-0029 extenders) was asked
21022+
// before the store delete, through `assertPackageUninstallable`, so it
21023+
// does not arrive here. The `catch` stays as a safety net for a
21024+
// registry that lacks that method, or a throw nothing asked ahead of
21025+
// time: it is logged, not fatal, because the durable row is already
21026+
// gone and the next boot is clean either way.
2100621027
try {
2100721028
(this.engine as any)?.registry?.uninstallPackage?.(request.packageId);
2100821029
} catch (e) {

‎packages/objectql/src/registry.ts‎

Lines changed: 53 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -3311,45 +3311,68 @@ export class SchemaRegistry {
33113311
}
33123312
}
33133313

3314+
/**
3315+
* [#21276] Would uninstalling `packageId` be refused? Throws the refusal
3316+
* {@link unregisterObjectsByPackage} (and so {@link uninstallPackage}) would
3317+
* raise, and returns normally otherwise. It reads `objectContributors` and
3318+
* mutates nothing, so a caller can ask before a step it cannot take back:
3319+
* `deletePackage` (`@objectstack/metadata-protocol`) asks it before it
3320+
* deletes the stored `sys_packages` row, so an uninstall this registry would
3321+
* refuse is refused with nothing removed.
3322+
*
3323+
* [#7970] THE REFUSAL PASS — the whole decision, taken before a single
3324+
* contribution is removed. This check used to live inline in the mutation
3325+
* walk of {@link unregisterObjectsByPackage}, one object at a time, so a
3326+
* package owning `account` (free) and `contact` (extended by another
3327+
* package) lost `account` on the way to refusing over `contact`: the guard
3328+
* that exists to keep a registry whole was itself reached through a
3329+
* mutation, and nothing rolled it back. Same predicate and same iteration
3330+
* order as the inline check it replaced, so the same object still refuses
3331+
* with the same message.
3332+
*
3333+
* ⛔ ONE predicate: {@link unregisterObjectsByPackage} calls this method
3334+
* rather than keeping its own copy, so the question asked ahead and the
3335+
* refusal raised by the uninstall cannot disagree. `force` is not a
3336+
* parameter here: forcing means not asking, and stays the caller's choice.
3337+
*
3338+
* @throws Error if the package owns an object another package extends (ADR-0029)
3339+
*/
3340+
assertPackageUninstallable(packageId: string): void {
3341+
for (const [fqn, contributors] of this.objectContributors.entries()) {
3342+
const ownedHere = contributors.some(
3343+
c => c.packageId === packageId && c.ownership === 'own'
3344+
);
3345+
if (!ownedHere) continue;
3346+
// Extenders from other packages
3347+
const otherExtenders = contributors.filter(
3348+
c => c.packageId !== packageId && c.ownership === 'extend'
3349+
);
3350+
if (otherExtenders.length > 0) {
3351+
throw new Error(
3352+
`Cannot uninstall package "${packageId}": object "${fqn}" is extended by ` +
3353+
`${otherExtenders.map(c => c.packageId).join(', ')}. Uninstall extenders first.`
3354+
);
3355+
}
3356+
}
3357+
}
3358+
33143359
/**
33153360
* Unregister all objects contributed by a package.
33163361
*
33173362
* [#7970] **Refuses before it mutates.** If any object this package owns is
33183363
* extended by another package (ADR-0029), the call throws having removed
3319-
* nothing — the refusal is decided across every object first. Callers may
3320-
* therefore treat a throw as a no-op, which is what lets
3321-
* {@link uninstallPackage} run this verb ahead of its own mutations.
3364+
* nothing — the refusal is decided across every object first, by
3365+
* {@link assertPackageUninstallable}. Callers may therefore treat a throw as
3366+
* a no-op, which is what lets {@link uninstallPackage} run this verb ahead of
3367+
* its own mutations.
33223368
*
33233369
* @throws Error if trying to uninstall an owner that has extenders
33243370
*/
33253371
unregisterObjectsByPackage(packageId: string, force: boolean = false): void {
3326-
// [#7970] REFUSAL PASS — the whole decision, taken before a single
3327-
// contribution is removed. This check used to live inline in the mutation
3328-
// walk below, one object at a time, so a package owning `account` (free)
3329-
// and `contact` (extended by another package) lost `account` on the way to
3330-
// refusing over `contact`: the guard that exists to keep a registry whole
3331-
// was itself reached through a mutation, and nothing rolled it back. Same
3332-
// predicate and same iteration order as the inline check it replaces, so
3333-
// the same object still refuses with the same message — what changed is
3334-
// only that no removal precedes the throw.
3335-
if (!force) {
3336-
for (const [fqn, contributors] of this.objectContributors.entries()) {
3337-
const ownedHere = contributors.some(
3338-
c => c.packageId === packageId && c.ownership === 'own'
3339-
);
3340-
if (!ownedHere) continue;
3341-
// Extenders from other packages
3342-
const otherExtenders = contributors.filter(
3343-
c => c.packageId !== packageId && c.ownership === 'extend'
3344-
);
3345-
if (otherExtenders.length > 0) {
3346-
throw new Error(
3347-
`Cannot uninstall package "${packageId}": object "${fqn}" is extended by ` +
3348-
`${otherExtenders.map(c => c.packageId).join(', ')}. Uninstall extenders first.`
3349-
);
3350-
}
3351-
}
3352-
}
3372+
// [#7970] REFUSAL PASS — see {@link assertPackageUninstallable}, which holds
3373+
// the one copy of the predicate ([#21276] extracted so it can be asked
3374+
// without uninstalling).
3375+
if (!force) this.assertPackageUninstallable(packageId);
33533376

33543377
// MUTATION PASS — carries no refusal of its own; the pass above already
33553378
// proved every removal below is allowed. Keep it that way: a second copy of

‎packages/runtime/src/domains/packages.ts‎

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2112,11 +2112,15 @@ export async function handlePackagesRequest(deps: DomainHandlerDeps, path: strin
21122112
// the uninstall, and its refusal (another package extends an object
21132113
// this one owns, ADR-0029) is the request's refusal.
21142114
//
2115-
// With a persisted half, that refusal now arrives AFTER the stored
2116-
// rows were deleted — the registry has no verb that answers it
2117-
// without performing it. It is reported as `registryRemoved: false`
2118-
// on the answer rather than as a failure the store does not bear
2119-
// out: the package leaves the running process at the next restart.
2115+
// With a persisted half, that refusal does not arrive here:
2116+
// `deletePackage` asks it (`SchemaRegistry.assertPackageUninstallable`)
2117+
// before its store delete and throws it with nothing removed, which
2118+
// the `catch` above answers — `500`, nothing changed. This `try`
2119+
// stays as a safety net for a registry without that method, or a
2120+
// throw nothing asked ahead of time. The stored rows are already
2121+
// gone by then, so it is reported as `registryRemoved: false`
2122+
// rather than as a failure the store does not bear out, and the
2123+
// package leaves the running process at the next restart.
21202124
if (registry.getPackage(id) !== undefined) {
21212125
if (persists) {
21222126
try {

0 commit comments

Comments
 (0)