Skip to content

Commit a96bf3c

Browse files
fix(objectql): a refused first install leaves nothing of itself, even over a tenant's object (#28839)
domain:server#2 · os-project-manager · session_0186vfjHHMsRXC9EUT9hTFMo <!-- claude-projects-attribution: {"github_login":"os-project-manager"} --> _Requested by **Leehom** · [project thread](https://claude.ai/code/project/chan_01TenzsRopZj4jMbvjX8uz1X?thread=cmsg_01TenzsRopZj4jMbvjX8uz1XJn3tHAnFh4h1yjpiKahdTG)_ Refs #28697 Clause-②: no Metadata-protocol: none ## 维护者速读 - **改了什么**:首次安装一个包时,如果注册表在注册过程中拒绝了它,这个包不会留下任何东西,包括它叠在租户已有对象(及租户的覆盖层)、无主覆盖层或别的包的扩展之上的情况;原来在那里的内容和尝试之前读起来完全一样。 - **为什么改**:被拒的安装应当让系统保持原样,运维修好包再装即可。拒绝本身(错误码、状态、文字)不变。 - **风险与回滚**:objectql patch。所有 `registerApp` 现在都经过注册表已有的"被拒即放回"(升级已经在用的那一套),首次安装不再走卸载式撤回;干净的安装、升级和运维卸载行为不变,禁用状态的启动种子照旧保留。回滚即 revert 本 PR。 - **先看**:`packages/objectql/src/engine.ts` 的 `registerApp`(一处调用替换了原来的分支和撤回路径);`registry-module-mock.ts` 的注释与成员;`registry.ts` 只改了 `uninstallPackage` 的文档注释。 - **两分钟验证**:`pnpm --filter @objectstack/objectql exec vitest run src/package-refused-first-install-over-tenant.test.ts src/package-refused-registration-withdrawn.test.ts`,预期 7 + 24 passed。 - **需要你做的**:无,席位落地。 ## What changes - **A refused first install leaves nothing of itself, in every case.** When `ObjectQL.registerApp` registers a package id for the first time and the registry refuses it part-way, no package record, object, extension, app, item, picklist extension, namespace, navigation contribution, contributed kind or datasource definition of the package remains. This now also holds when the package layers over an object a tenant already holds (with or without the tenant's overlay layer), over an overlay with no owner, or over another package's extension of one of its objects. What was there reads exactly as before: who holds each layer, how it is classified, its priority, its body, and the object as served. - **One mechanism.** Every registration body now runs through the registry's existing `SchemaRegistry.restoringOnRefusal`, the restore a refused upgrade already uses. The engine's separate uninstall-based withdrawal of a refused first install, and the check that decided whether it was safe, are removed: the restore covers every case the withdrawal did, and the cases it could not take. - **Unchanged:** what is refused and every refusal's code, status and words; a clean install, a clean upgrade and a refused upgrade; an operator's uninstall; a persisted "disabled" state for the id still applies to its next install. `SchemaRegistry.uninstallPackage`'s `keepBootSeed` option stays available and unchanged; only its doc comment changes, since the engine no longer calls it for a refused install. - The shared registry test double's comment now says what `registerApp` reads from it; one member nothing reads any more is dropped from it. - Changeset: `patch` for `@objectstack/objectql`. It carries a sentence superseding the pending changeset line that said the engine passes `keepBootSeed` when it withdraws a refused first install. ## Tests - `packages/objectql/src/package-refused-first-install-over-tenant.test.ts` (new, 7 tests). Refused first installs of a package that layers over a tenant's object (with or without the tenant's overlay), over an overlay with no owner, or over another package's extension. Each asserts the same code and status, that nothing of the package remains, and that the existing object reads exactly as before the attempt. A fifth case keeps the boot seed of disabled packages. Controls: a clean first install over a tenant's object takes ownership with the tenant's layer as its overlay, as before; an operator's uninstall of it still removes the package. - `package-refused-registration-withdrawn.test.ts`: the control for a body that took over a tenant's object now asserts the behaviour (nothing of the package, the tenant's object owned and served as before). Every other case in the file is unchanged. ## Acceptance notes - Through the HTTP doors, the tenant case is pinned at the unit layer only; the existing install-local dogfood file (first installs and upgrades refused part-way) runs green on this engine. - Reading every open PR touching `objectql/src/engine.ts` or `registry.ts` before opening this one: only #28811 touches `registry.ts` (the `extend` refusal block and a new removal verb), away from these hunks; `git merge-tree` against it and against `origin/main` is clean. ## Verification All readings below are on the tree of `c10fd4ae` (the same tree as the commit the runs were taken on). <details><summary>Red first on base (`693e1003`)</summary> - The new file on base: 5 failed, 2 passed. Each of the five passed its code-and-status assertion and failed at the package-state comparison; the two controls pass on base. </details> <details><summary>Green</summary> - New file 7/7; `package-refused-registration-withdrawn.test.ts` 24/24; `package-refused-upgrade-keeps-installed.test.ts` 13/13; `engine.test.ts` green (180 tests across these four files). - `pnpm --filter @objectstack/objectql exec vitest run`: 522 files, 10010 tests, all green (`VERDICT command-exit 0`). - `pnpm --filter @objectstack/objectql typecheck`: exit 0 (`check:test-typecheck` included). - Dogfood, with the `@objectstack/dogfood^...` closure built under the lock (objectql dist carrying the new call, the removed path's log line absent): `install-local-refused-package-leaves-nothing` 4/4. </details> <details><summary>Ablation (unit files; objectql tests import src), through ablation-replace, restore proven blob == HEAD and `git diff HEAD` empty</summary> - The restore limited to installed ids (first installs registered bare, no withdrawal): 11 failed, 33 passed over the three registration files. Red: the five new cases, the flipped tenant control, and the five first-install registry cases of the withdrawal file, which this call now carries. The upgrade file stays 13/13. </details> <details><summary>Derived gates (64)</summary> - 63 exit 0. `check:dual-build-cjs-loads` exit 3: NOT MEASURED, it needs every package's dist (a full `pnpm build`). `check:dts-closure` and the teardown self-test were re-run after the dist closure finished and the self-test's pinned fixture was fetched; both exit 0. `dispatch-gates --ran`: 64 accounted, 63 run, 1 NOT MEASURED. - CI runs the full farm on this PR. </details> 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01FEkQ5oL2i4xH7Mu7va3kxr --- _Generated by [Claude Code](https://claude.ai/code/session_01FEkQ5oL2i4xH7Mu7va3kxr)_ Co-authored-by: Claude <noreply@anthropic.com>
1 parent fa59f78 commit a96bf3c

6 files changed

Lines changed: 233 additions & 147 deletions
Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
---
2+
"@objectstack/objectql": patch
3+
---
4+
5+
Clause-②: no
6+
Metadata-protocol: none
7+
8+
fix(objectql): a refused first install leaves nothing of itself, even over a tenant's object
9+
10+
A first install of a package that `ObjectQL.registerApp` refuses part-way now leaves nothing of itself registered in every case, including when the package layers over an object a tenant already holds (with or without the tenant's own overlay), over an overlay with no owner, or over another package's extension of one of its objects. Afterwards there is no package record, object, extension, app, item, picklist extension, namespace, navigation contribution, contributed kind or datasource definition of the package, and the tenant's object, its overlay and the other package's extension read exactly as before the attempt. A persisted "disabled" state for that id still applies to its next install. What is refused, and every refusal's code, status and words, is unchanged.
11+
12+
Every registration is now put back by the registry itself when it is refused, the way a refused upgrade already was. This supersedes the line of the pending `28697-refused-registration-leaves-nothing` changeset saying the engine passes `{ keepBootSeed: true }` to `SchemaRegistry.uninstallPackage`: the engine no longer uninstalls a refused first install. The option itself stays available and unchanged.

‎packages/objectql/src/engine.ts‎

Lines changed: 12 additions & 122 deletions
Original file line numberDiff line numberDiff line change
@@ -331,10 +331,7 @@ import {
331331
} from './home-amount-projection.js';
332332
import { assertAggregatedColumnsAreDistinct, assertDateBucketTargetsAreTemporal } from './aggregate-query-shape-door.js';
333333
import { applyProtection, pluralToSingular, ExternalWriteForbiddenError } from '@objectstack/spec/shared';
334-
import { SchemaRegistry, computeFQN, type ArtifactInstallScope } from './registry.js';
335-
// [#28697] Whether an object's sitting owner is a tenant's own (ADR-0029 D9.6),
336-
// asked by `refusedRegistrationWithdrawable` before a refused body is withdrawn.
337-
import { isTenantAuthored } from '@objectstack/metadata-core';
334+
import { SchemaRegistry, type ArtifactInstallScope } from './registry.js';
338335
import { expandSearchToFilter, normalizeSearch, SEARCHABLE_ENUM_TYPES, SEARCHABLE_TEXTUAL_TYPES } from './search-filter.js';
339336
import { stripSearchCompanion } from './search-companion.js';
340337
import { ExpressionEngine, analyzeRelationshipTraversals, isUnparseableDate, type EvalError, type EvalOptionLabels, type EvalPermissions } from '@objectstack/formula';
@@ -5176,65 +5173,6 @@ export function assembledViewItemRefusal(
51765173
return err;
51775174
}
51785175

5179-
/**
5180-
* [#28697] Whether a refused {@link ObjectQL.registerApp} of package `id` may
5181-
* be withdrawn through the registry's uninstall, asked BEFORE anything of the
5182-
* body is written. True only when withdrawing takes back exactly what the
5183-
* refused call wrote and nothing else:
5184-
*
5185-
* - no package record answers `id` yet. A refused re-register of an
5186-
* installed id is NOT withdrawn: the uninstall would take the installed
5187-
* version with it;
5188-
* - `id` is not the owner a stack's residual registers its items under
5189-
* (`recordResidualOwner`), whose items no package record covers;
5190-
* - no object the body or a nested plugin owns, nor any object it extends,
5191-
* already holds a contribution under `id`;
5192-
* - every object the body or a nested plugin owns is either unheld or owned
5193-
* by another code package (which `registerObject` refuses before it adds
5194-
* anything). A tenant-authored owner or an overlay or extension with no
5195-
* owner is left alone: `registerObject` re-classifies the tenant
5196-
* contribution as the package's overlay, and the uninstall drops an
5197-
* object's overlay with its owner (ADR-0029 D9.7) and refuses an object
5198-
* another package extends, so withdrawing would lose the tenant's layer in
5199-
* this process or be refused.
5200-
*/
5201-
export function refusedRegistrationWithdrawable(
5202-
registry: SchemaRegistry,
5203-
id: string | undefined,
5204-
manifest: any,
5205-
residualOwners: ReadonlyMap<string, unknown>,
5206-
): boolean {
5207-
if (!id || registry.getPackage(id) !== undefined || residualOwners.has(id)) return false;
5208-
// The object names as the registry files them (`computeFQN`, the call
5209-
// `registerObject` makes with the namespace each one registers under).
5210-
const owned: string[] = [];
5211-
const ownedNames = (body: any, namespace: string | undefined) => {
5212-
const objects = body?.objects;
5213-
if (!objects || typeof objects !== 'object') return;
5214-
const names = Array.isArray(objects) ? objects.map((def: any) => def?.name) : Object.keys(objects);
5215-
for (const name of names) if (typeof name === 'string' && name !== '') owned.push(computeFQN(namespace, name));
5216-
};
5217-
ownedNames(manifest, manifest?.namespace);
5218-
if (Array.isArray(manifest?.plugins)) {
5219-
for (const plugin of manifest.plugins) {
5220-
if (plugin && typeof plugin === 'object') ownedNames(plugin, plugin.namespace || manifest?.namespace);
5221-
}
5222-
}
5223-
const extended: string[] = Array.isArray(manifest?.objectExtensions)
5224-
? manifest.objectExtensions.map((ext: any) => ext?.extend).filter((name: unknown): name is string => typeof name === 'string')
5225-
: [];
5226-
for (const fqn of [...owned, ...extended]) {
5227-
if (registry.getObjectContributors(fqn).some((c) => c.packageId === id)) return false;
5228-
}
5229-
for (const fqn of owned) {
5230-
const contributors = registry.getObjectContributors(fqn);
5231-
if (contributors.length === 0) continue;
5232-
const ownedByAnotherCodePackage = contributors.some((c) => c.ownership === 'own' && !isTenantAuthored(c.definition));
5233-
if (!ownedByAnotherCodePackage) return false;
5234-
}
5235-
return true;
5236-
}
5237-
52385176
/**
52395177
* [#28697] Stamp an app object ({@link stampPackage}) and register it through
52405178
* the registry's app verb, recording the stamp the registration writes onto
@@ -9324,28 +9262,16 @@ export class ObjectQL implements IObjectQLEngine {
93249262

93259263
// [#28697] What remains can only be refused part-way, by the registry's
93269264
// own verbs (an object another package owns, a field with no `type`, a
9327-
// picklist value already added, …). A first registration of this id is
9328-
// withdrawn when one throws, so the caller's refusal leaves no package
9329-
// record, object, app or item behind; asked here, before anything is
9330-
// written. See {@link refusedRegistrationWithdrawable}.
9331-
const withdrawable = refusedRegistrationWithdrawable(this._registry, id, manifest, this.residualOwnerSources);
9332-
// [#28697] A re-register of an installed id is an upgrade: refused
9333-
// part-way, the registry puts back exactly the entries it wrote, so the
9334-
// installed version keeps serving whole, with other packages'
9335-
// extensions of its objects and tenant overlays as they were. Never
9336-
// through uninstall, which refuses an extended object and drops a
9337-
// tenant's overlay. See {@link SchemaRegistry.restoringOnRefusal}.
9338-
const upgrade = Boolean(id) && this._registry.getPackage(id) !== undefined;
9339-
try {
9340-
if (upgrade) {
9341-
this._registry.restoringOnRefusal(() => this.registerPackageBody(manifest, id, namespace, version, source, scope));
9342-
} else {
9343-
this.registerPackageBody(manifest, id, namespace, version, source, scope);
9344-
}
9345-
} catch (err) {
9346-
if (withdrawable) this.withdrawRefusedRegistration(id, manifest, err);
9347-
throw err;
9348-
}
9265+
// picklist value already added, …). Refused, the registry puts back
9266+
// exactly the entries this registration wrote: a first install leaves no
9267+
// package record, object, app, item or namespace of itself, and a
9268+
// re-register of an installed id (an upgrade) leaves the installed
9269+
// version serving whole. Whatever sat there before — a tenant's object
9270+
// or overlay this package layers over (ADR-0029 D9 §6.1), another
9271+
// package's extension of its objects — reads as it did. Never through
9272+
// uninstall, which refuses an extended object and drops a tenant's
9273+
// overlay with its owner (D9.7). See {@link SchemaRegistry.restoringOnRefusal}.
9274+
this._registry.restoringOnRefusal(() => this.registerPackageBody(manifest, id, namespace, version, source, scope));
93499275

93509276
// [#28697] Written only once nothing above threw, so a refused body
93519277
// leaves none of them. Nothing above reads any of them.
@@ -9524,7 +9450,7 @@ export class ObjectQL implements IObjectQLEngine {
95249450
}
95259451

95269452
// (Contributed kinds register in `registerApp` once this body has
9527-
// registered whole: they belong to no package, so no withdrawal of a
9453+
// registered whole: they belong to no package, so no restore of a
95289454
// refused body could take them back.)
95299455

95309456
// 7. Recursively register nested plugins
@@ -9540,42 +9466,6 @@ export class ObjectQL implements IObjectQLEngine {
95409466
}
95419467
}
95429468

9543-
/**
9544-
* [#28697] Take back what a refused first registration of package `id`
9545-
* wrote, through the registry's own uninstall ({@link SchemaRegistry.uninstallPackage}):
9546-
* the package record and its namespace, its objects and extensions, its apps
9547-
* and items, its picklist extensions and catalog claims. Two things that verb
9548-
* does not reach are handled here: a nested plugin's own namespace (claimed
9549-
* for `id` by `registerObject`), and the boot seed, which an operator
9550-
* uninstall forgets with the row but a refused registration keeps, so a later
9551-
* install of the id still lands disabled when the persisted state says so.
9552-
*
9553-
* Nothing to do when the registry refused the package record itself: its
9554-
* refusals all precede its first write. A withdrawal that is itself refused
9555-
* is logged and the caller's refusal still propagates unchanged.
9556-
*/
9557-
private withdrawRefusedRegistration(id: string, manifest: any, refusal: unknown): void {
9558-
if (this._registry.getPackage(id) === undefined) return;
9559-
try {
9560-
this._registry.uninstallPackage(id, { keepBootSeed: true });
9561-
if (Array.isArray(manifest.plugins)) {
9562-
for (const plugin of manifest.plugins) {
9563-
const ns = plugin && typeof plugin === 'object' ? plugin.namespace : undefined;
9564-
if (typeof ns === 'string' && ns !== '' && ns !== manifest.namespace) this._registry.unregisterNamespace(ns, id);
9565-
}
9566-
}
9567-
this.invalidateSummaryIndex();
9568-
this.logger.debug('Withdrew a refused package registration', { id });
9569-
} catch (withdrawal) {
9570-
this.logger.error(
9571-
`Package '${id}' was refused part-way through registering, and what it had registered could not be `
9572-
+ 'withdrawn; it stays in this process until restart.',
9573-
withdrawal instanceof Error ? withdrawal : undefined,
9574-
{ id, refusal: String((refusal as Error)?.message ?? refusal), withdrawal: String((withdrawal as Error)?.message ?? withdrawal) },
9575-
);
9576-
}
9577-
}
9578-
95799469
/**
95809470
* Deep-clone an app definition, resolving objectName references in navigation
95819471
* items via the registry. Object names are canonical identifiers — no FQN

0 commit comments

Comments
 (0)