Skip to content

Commit bafbc68

Browse files
fix(objectql): a refused member of a multi-package artifact leaves no member of it registered (#31564) (#31614)
<!-- claude-projects-attribution: {"github_login":"os-steve"} --> _Requested by **Steve** · [project thread](https://claude.ai/code/project/chan_01FdL1XeUyvVNxLreh3F4Qi9?thread=cmsg_01FdL1XeUyvVNxLreh3F4Qi934LugcHrYQZvw8K1SD7K3S)_ domain:server#2 · os-steve · session_01ADHkmPsKvRzUCdA1j8gTUX Fixes #31564 Clause-②: yes (widening) — adds one public method, `SchemaRegistry.recordRestoreOnRefusal` (re-exported through `@objectstack/objectql`'s index), and refuses nothing that registers today. Metadata-protocol: none ## 维护者速读 - 改了什么:引擎的 `manifest` 服务把一个多包产物的所有成员包(以及产物的"残余"顶层项)放进同一个注册还原事务里注册。任何一处被拒,所有成员写过的东西一起撤回:注册表条目、导航贡献、引擎自己的 manifest 表和数据源定义表。 - 为什么改:以前每个包只撤回自己。后面的成员被拒时,前面已注册的成员留在运行中的内核里;安装接口回 422、不写安装记录,重启后又消失。首装时内核在服务一个被拒的安装,升级时第一个成员变成了被拒的 2.0.0 而第二个还是 1.0.0。 - 风险与回滚:拒绝的判定一个没改,接受和拒绝的范围与今天完全一样,只是拒绝后内核状态回到产物到达之前。新增一个注册表方法 `recordRestoreOnRefusal`,让引擎把自己表里的写入登记到同一个还原事务。回滚即还原本 PR。 - 先看:`packages/objectql/src/plugin.ts`(`manifest.register` 里包住整个产物的那一段),`packages/objectql/src/engine-registration.ts`(`registerApp` 登记 manifest 和数据源定义),`packages/objectql/src/registry.ts`(`recordRestoreOnRefusal` 和导航贡献的登记)。 - 两分钟验证:`pnpm --filter @objectstack/objectql exec vitest run src/manifest-service-artifact-refusal-restores.test.ts`,预期 10 个全绿。 - 需要你做的:无,席位落地。 ## What changes **The choice: (a) in its strongest form — one restore run spans the whole artifact.** `manifest.register` (`packages/objectql/src/plugin.ts`) now runs every package body of the artifact, and then its residual, inside one `SchemaRegistry.restoringOnRefusal` run. Each `registerApp` already opens such a run for its own body; a nested call joins the open run, so a refusal anywhere in the artifact puts back every entry ANY member wrote, and the refusal reaches the caller unchanged (same code, status and sentence). What a refused artifact now leaves: - **First install:** no member registered — no package record, object, app, page, navigation contribution, kind, datasource definition, namespace claim or engine manifest entry of any member. - **Upgrade:** the installed version of EVERY member keeps serving, whole. On `main` the first member became the refused `2.0.0` while the second stayed `1.0.0`. - **A residual refusal** (top-level items no body declares, registered after every body): the same — no member registered. To make "every entry" true, three writes that sat outside the run are now recorded in it: 1. `packages/objectql/src/registry.ts` — each app's list of navigation contributions (`setPackageNavContributions`, `registerAppNavContribution`), recorded before its first write in a run. 2. `packages/objectql/src/registry.ts` — `recordRestoreOnRefusal(key, capture)`, a new public method: an engine map a package body writes beside the registry is recorded in the open run, the way the registry records its own entries. Outside a run it records nothing. 3. `packages/objectql/src/engine-registration.ts` — `registerApp` records, through it, the engine's `manifests` entry and each datasource definition (restored with the driver floor `applyExternalDriverFloor` decides), and drops the summary index again on restore. A single-package artifact is the same one-body run as before: outside a multi-member run nothing new is recorded, and every refusal is raised at the same place. One ordering change in `plugin.ts`: the residual owner's origin (`ObjectQL.recordResidualOwner`) is now recorded once the artifact has registered whole, not before. A recorded `marketplace` origin is never taken back, so recording it for a refused artifact left a stale origin for an id nothing registered. No reader runs during registration (searched: the only reader is `service-automation`'s, after the door answers), and the bridge that follows still sees it. ### Why (a) in this form, and not (b), on the four axes - **Real business need.** The reach is every install-local of a multi-package artifact (inline, and the cloud branch since #31563) whose later member the engine refuses, and every boot of such a stack. The measured symptom is a kernel that serves an install it answered `422` for, until the next restart. Both branches measured red on `main` (below). - **Long-term soundness.** (b) "check the whole artifact before any member registers" cannot see the refusals that matter: most are raised by the registry's own verbs part-way, against state the earlier members just wrote (the artifact's own object-name conflict reads member one's object; the ADR-0130 D1 co-owner gate reads member one's namespace). A pre-check would have to re-implement every verb's judge against a simulated registry — the second copy ADR-0130's comment on this door forbids. Literal (a), "withdraw the members already registered", would go through `uninstallPackage`, which refuses an object another package extends, drops a tenant's overlay with its owner (ADR-0029 D9.7), and cannot put an upgrade's installed version back. The restore run (#28697) already exists for exactly this, records only what the run writes, and already restores an upgrade whole for one body; spanning it over the artifact is the root fix, not a second mechanism. - **Preventing AI mistakes.** The refusal stays loud and unchanged; only what is left behind changes. No lenient path is added and nothing is tolerated that was refused before. An AI author retrying a corrected artifact meets a clean kernel instead of half of its previous attempt. - **Startup focus.** No new door, no new authorable surface, no spec change. One new public method on `SchemaRegistry` (listed in open questions for the seat). ### Forced test edits - `packages/objectql/src/registry-artifact-co-ownership.test.ts` — "leaves NOTHING behind when it refuses": it read `com.acme.crm` (the member registered before the refused one) as still registered, still the `crm` namespace owner and still `crm_account`'s owner. Those three reads were the defect. It now asserts neither member, no namespace owner and no definition of `crm_account`. - `packages/cloud-connection/src/marketplace-install-local-register-refusal.test.ts` — the "object name two packages of one artifact define" case listed only the refused member as unregistered, with a comment saying the earlier one stays and is "not pinned here". It now lists both members (the cloud-branch pin), and a new inline-branch case sits beside it in the same harness. - `packages/objectql/src/registry-module-mock.ts` — the registry double gains `recordRestoreOnRefusal: vi.fn()`; an omitted member is a hard `TypeError` at `registerApp`'s first call. ## Tests New `packages/objectql/src/manifest-service-artifact-refusal-restores.test.ts` (10 cases, real `ObjectKernel` + `ObjectQLPlugin`, artifacts arriving after `start()` through the `manifest` service): - fresh install, second member refused by a registry verb part-way (an object another installed package owns), by the artifact's own object-name conflict, and by a body-only judge (an item wearing another package's `_packageId`): neither member registered, the refusal keeps its code and status, the holder unchanged; - a refusal the residual raises after both members registered: neither registered, and no residual-owner origin recorded; - upgrade `1.0.0` → `2.0.0`, second member refused (the same three ways): the trace (packages, objects with labels, apps, pages, navigation, kinds, datasource definitions, engine manifests and their versions, namespaces) reads exactly as before; - CONTROLS: a single-package refusal answers exactly what a bare `registerApp` answers (code, status, message) and leaves nothing; a clean two-member artifact registers both; a clean artifact with a residual records the residual owner's origin. Cloud-connection, in the #31478 harness: the cloud-branch case now pins both members unregistered; a new inline-branch case pins the same. ### Red on main first (645ed3d, objectql and cloud-connection src unchanged) <details><summary>objectql: 7 failed | 2 passed (9) — the controls are the passes</summary> ``` × a fresh install whose second member is refused: an object another installed package owns (a registry verb, part-way) × a fresh install whose second member is refused: an object name the first member also defines (the artifact's own conflict) × a fresh install whose second member is refused: a page wearing another package's `_packageId` (judged from the body alone) × a refusal the artifact's residual raises, after both members registered × the second member of 2.0.0 is refused: (the same three) Tests 7 failed | 2 passed (9) ``` Fresh install: `"packages": ["com.probe.suite.core@1.0.0", "com.probe.suite.addon@-"]` with core's object, app, page, navigation contribution, kind, datasource definition and engine manifest still present. Upgrade: core read `@2.0.0` (`suite_core_thing:…:core 2.0.0`, `suite_nav_2_0_0`, `suite.kind.2.0.0`) while the addon read `@1.0.0`. (The residual-owner pin was added after this run; leg 4 below is its red.) </details> <details><summary>cloud-connection: 2 failed | 10 passed (12) — both branches</summary> ``` × a fresh install of an object name two packages of one artifact define is refused: 422 with the engine's sentence, nothing registered or ledgered AssertionError: com.acme.crm: expected { manifest: { …(6) }, …(6) } to be undefined × the inline branch: a multi-package artifact whose second member is refused leaves neither member registered AssertionError: expected { manifest: { …(6) }, …(6) } to be undefined Tests 2 failed | 10 passed (12) ``` The 10 passes include every single-package refusal case (the CONTROL: answered as today). </details> ### Ablations (fix committed first; `scripts/ablation-replace.mjs`, restore proven blob == HEAD each leg) Tests resolve `@objectstack/objectql` from `src` (objectql's own suite imports `./plugin.js`; cloud-connection aliases `@objectstack/objectql` to `../objectql/src/index.ts`), so no `dist` leg is involved. <details><summary>four legs, each red, each restored</summary> 1. **The artifact run removed** — `plugin.ts` `ql.registry.restoringOnRefusal(() => {` → `((run: () => void) => run())(() => {` (blob `b1ac4873` → `30e786cd`): objectql `Tests 8 failed | 17 passed (25)` (the 7 refusal pins + the co-ownership pin), cloud-connection `Tests 2 failed | 10 passed (12)` (cloud + inline). Restored `b1ac4873` == HEAD. 2. **Outside entries not recorded** — `registry.ts` `recordRestoreOnRefusal` body → `void key; void capture;` (blob `9d12f869` → `ae44483e`): `Tests 7 failed | 3 passed (10)`, the diffs on `datasources` and `manifests`. Restored == HEAD. 3. **Navigation contributions not recorded** — `registry.ts` `noteAppNavContributions` made to skip its note (`this.restorePoint` → `(app ? undefined : this.restorePoint)`) (blob `9d12f869` → `ad4684e2`): `Tests 7 failed | 3 passed (10)`, the diff on `nav` (`com.probe.suite.core:suite_nav_1_0_0` left behind). Restored == HEAD. 4. **Residual owner recorded before the run** (main's order) — (blob `b1ac4873` → `c44f3509`): `Tests 1 failed | 9 passed (10)`, `expected 'marketplace' to be undefined`. Restored == HEAD. A first attempt at legs 3 and 4 was refused by the tool before running anything (the replacement contained the anchor, so the anchor count did not drop); re-planted with replacements that do not contain it. </details> ## Acceptance notes - `ObjectQL.recordResidualOwner`'s record is sticky by design (`marketplace` never turns into `code`); this PR only stops writing it for a refused artifact. - Bridging into the metadata service already ran after all registration, so a refused artifact never reached the metadata service on `main` either; unchanged. ## Verification Head **`dcec15bd`**: `6cc7467a` (fix + pins, merged with `origin/main` at `6b83ec2d`; the merge touched neither objectql nor cloud-connection) plus one commit that changes only the changeset's `Clause-②:` line. The readings below were taken on `6cc7467a`; on `dcec15bd` the changeset families were re-run: `check-adr-0087-registration --base origin/main`, `check-changeset-no-major --base origin/main`, `check-empty-changeset --base origin/main`, `check:changeset-gate-self-tests`, `check:closed-ceremony-text`, each exit 0. - `turbo run build --concurrency=2 --filter='./packages/**'` after the merge: `Tasks: 81 successful, 81 total` (os-verify-lock VERDICT command-exit 0). - `pnpm --filter @objectstack/objectql test`: `Test Files 571 passed (571)`, `Tests 11581 passed (11581)`. - `pnpm --filter @objectstack/cloud-connection test`: `Test Files 78 passed (78)`, `Tests 1066 passed (1066)`. - `pnpm --filter @objectstack/objectql typecheck` and `pnpm --filter @objectstack/cloud-connection typecheck`: exit 0 (same lock run, VERDICT command-exit 0). - `node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack`: 72 families, all exit 0 (`check-plugin-teardown-shape --self-test` first exit 3 for its pinned fixture commit, exit 0 after fetching it). `--ran`: `72 derived famil(ies) accounted for — 72 run, 0 NOT-MEASURED`. - `pnpm exec eslint --no-inline-config packages/objectql packages/cloud-connection`: exit 0 (neither package declares a `lint` script). - Dogfood: not owed locally (no `!` in the title, no narrowing arm); the queue's Dogfood Regression Gate runs it. - `test:repo`: not run locally (the diff touches no repo-project test); CI's Test Core carries it. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01ADHkmPsKvRzUCdA1j8gTUX <!-- signed: domain:server#2 · os-steve · session_01ADHkmPsKvRzUCdA1j8gTUX --> --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 6924440 commit bafbc68

8 files changed

Lines changed: 452 additions & 46 deletions
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
---
2+
'@objectstack/objectql': patch
3+
---
4+
5+
fix(objectql): a refused member of a multi-package artifact leaves no member of it registered (#31564)
6+
7+
Clause-②: yes (widening) — adds one public method, `SchemaRegistry.recordRestoreOnRefusal`, and refuses nothing that registers today.
8+
Metadata-protocol: none
9+
10+
The engine's `manifest` service registered an artifact's packages one by one, and each package put back only what it had written itself when it was refused. When a later member was refused, the members registered before it stayed in the running kernel (package records, objects, apps, navigation contributions, kinds, datasource definitions), while the marketplace install door answered `422 PLUGIN_REGISTER_FAILED` and ledgered nothing, so a restart dropped them again.
11+
12+
- **Now.** Every package of one artifact, and then its residual (top-level items no package declares), registers in one restore run. A refusal anywhere in it puts back every entry any of them wrote, in the registry and in the engine's own manifest and datasource-definition maps, and the refusal reaches the caller unchanged. A first install leaves no member registered; a refused upgrade leaves the installed version of every member serving.
13+
- **Unchanged.** Nothing that registers today is refused: the same bodies are accepted and refused, with the same code, status and sentence. A single-package artifact is answered exactly as before. The residual owner's origin is now recorded once the artifact has registered whole, so a refused artifact records none.

‎packages/cloud-connection/src/marketplace-install-local-register-refusal.test.ts‎

Lines changed: 23 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,8 @@
3535
* bound action whose object nobody registers: `422 PLUGIN_REGISTER_FAILED`
3636
* in the declared envelope with the engine's sentence, nothing registered,
3737
* the ledger exactly as before the call;
38+
* - a multi-package artifact whose second member is refused, on the cloud
39+
* and on the inline branch: neither member registered (#31564);
3840
* - an upgrade refused by a registry refusal and by a sealed-vocabulary one:
3941
* the same `422`, the installed version still registered and ledgered;
4042
* - the remedy a sealed-vocabulary refusal names: once the package that
@@ -279,12 +281,11 @@ const CASES = [
279281
},
280282
{
281283
title: 'an object name two packages of one artifact define',
282-
// Only the refused member: the manifest service registers an artifact's
283-
// packages one by one, so `com.acme.crm`, registered before
284-
// `com.acme.crm.billing` was refused, stays in the RUNNING kernel —
285-
// unledgered, so gone at the restart. Measured the same on the inline
286-
// branch; not this door's to withdraw, and not pinned here.
287-
versionId: 'v1-artifact-object', manifestId: 'com.acme.crm-suite', registered: ['com.acme.crm.billing'],
284+
// BOTH members: `com.acme.crm` registered before `com.acme.crm.billing`
285+
// was refused, and the engine's `manifest` service puts back every
286+
// member of a refused artifact, so the running kernel holds neither —
287+
// as the ledger holds neither. The inline branch: below.
288+
versionId: 'v1-artifact-object', manifestId: 'com.acme.crm-suite', registered: ['com.acme.crm', 'com.acme.crm.billing'],
288289
says: 'Object name conflict inside one release artifact: object "crm_account"',
289290
},
290291
{
@@ -343,6 +344,22 @@ describe('install-local cloud branch refuses a register the engine refuses, befo
343344
});
344345
}
345346

347+
it('the inline branch: a multi-package artifact whose second member is refused leaves neither member registered', async () => {
348+
const { ql, install, installHolder } = await bootPlugin();
349+
await installHolder();
350+
const before = ledger();
351+
352+
const res = await install({ manifest: collidingArtifact() });
353+
354+
expect(res.status, JSON.stringify(res.payload)).toBe(422);
355+
expect(res.payload.error.code).toBe('PLUGIN_REGISTER_FAILED');
356+
expect(res.payload.error.message).toContain('Object name conflict inside one release artifact: object "crm_account"');
357+
expect(ql.registry.getPackage('com.acme.crm')).toBeUndefined();
358+
expect(ql.registry.getPackage('com.acme.crm.billing')).toBeUndefined();
359+
expect(ql.registry.getObjectOwner('crm_account')).toBeUndefined();
360+
expect(ledger()).toEqual(before);
361+
});
362+
346363
it('the remedy a bound action\'s refusal names: once the object\'s package is installed, the same install succeeds at once', async () => {
347364
serveVersions(VERSIONS);
348365
const { ql, installFromCloud, installHolder } = await bootPlugin();

‎packages/objectql/src/engine-registration.ts‎

Lines changed: 30 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,7 @@ export interface RegistrationHost {
8383
/** The bodies of the "Package, app, plugin, driver and datasource registration" section of `ObjectQL`: a stateless facet, called by the class's stubs. */
8484
export class RegistrationFacet {
8585
/** The body of `ObjectQL#registerApp`; the method's docblock there is its contract. */
86-
static registerApp(this: Pick<RegistrationHost, '_registry' | 'convertPackageBodyForward' | 'invalidateSummaryIndex' | 'logger' | 'manifests' | 'registerDatasourceDef' | 'registerPackageBody'>, registeredBody: any, scope?: ArtifactInstallScope, source: Exclude<InstalledPackageSource, 'authored'> = 'code'): void {
86+
static registerApp(this: Pick<RegistrationHost, '_registry' | 'applyExternalDriverFloor' | 'convertPackageBodyForward' | 'datasourceDefs' | 'invalidateSummaryIndex' | 'logger' | 'manifests' | 'registerDatasourceDef' | 'registerPackageBody'>, registeredBody: any, scope?: ArtifactInstallScope, source: Exclude<InstalledPackageSource, 'authored'> = 'code'): void {
8787
// The CONVERTED body, for everything below — the package record, the
8888
// live manifest, every item it registers (see {@link convertPackageBodyForward}).
8989
// [#23972] Refused before anything registers — see the docblock: an
@@ -143,6 +143,14 @@ export class RegistrationFacet {
143143
}
144144
}
145145
this.invalidateSummaryIndex(); // new objects may add/change summary fields
146+
// [#31564] Inside a run another caller opened — the `manifest` service
147+
// registers every package of one artifact in one run — a refusal of a
148+
// LATER package puts this one back too. Its writes outside the registry
149+
// are recorded for that run below, before each first write; the summary
150+
// index, dropped above, is dropped again once the registry is put back.
151+
// Outside such a run nothing is recorded (see
152+
// {@link SchemaRegistry.recordRestoreOnRefusal}).
153+
this._registry.recordRestoreOnRefusal('summaryIndex', () => () => this.invalidateSummaryIndex());
146154
this.logger.debug('Registering package manifest', { id, namespace });
147155

148156
// [#28697] What remains can only be refused part-way, by the registry's
@@ -163,6 +171,14 @@ export class RegistrationFacet {
163171

164172
// Store manifest for defaultDatasource lookup
165173
if (id) {
174+
this._registry.recordRestoreOnRefusal(`manifest\u0000${id}`, () => {
175+
const had = this.manifests.has(id);
176+
const held = this.manifests.get(id);
177+
return () => {
178+
if (had) this.manifests.set(id, held);
179+
else this.manifests.delete(id);
180+
};
181+
});
166182
this.manifests.set(id, manifest);
167183
}
168184

@@ -174,7 +190,19 @@ export class RegistrationFacet {
174190
? manifest.datasources
175191
: Object.entries(manifest.datasources).map(([name, def]) => ({ name, ...(def as any) }));
176192
for (const ds of dsList) {
177-
if (ds?.name) this.registerDatasourceDef(ds);
193+
if (!ds?.name) continue;
194+
const name: string = ds.name;
195+
// Put back with the definition: the driver floor it decides
196+
// (`applyExternalDriverFloor`, which `registerDatasourceDef` runs).
197+
this._registry.recordRestoreOnRefusal(`datasourceDef\u0000${name}`, () => {
198+
const held = this.datasourceDefs.get(name);
199+
return () => {
200+
if (held) this.datasourceDefs.set(name, held);
201+
else this.datasourceDefs.delete(name);
202+
this.applyExternalDriverFloor(name);
203+
};
204+
});
205+
this.registerDatasourceDef(ds);
178206
}
179207
}
180208

0 commit comments

Comments
 (0)