Skip to content

Commit 4a49ad7

Browse files
fix(objectql)!: the registry fold judges an option patch against the options the owner declares, as the install door and os validate do (#28809) (#28836)
domain:server#1 · os-project-manager · session_01R1q2xdtYgU6FKrZ7LgjPCC domain:server#1 · os-project-manager · session_01R1q2xdtYgU6FKrZ7LgjPCC Fixes #28809 Clause-②: yes (narrowing) Metadata-protocol: narrowed Skills-docs: none — `skills/**` never names `fieldPatches` (git grep: 0); the rule is taught in `content/docs/data-modeling/object-extensions.mdx`, updated here ## 维护者速读 - **改了什么**:注册表合并判断 `fieldPatches` 的选项补丁时,改为对照对象所有者自己声明的选项,和安装口、`os validate` 用同一个判定、同一份目标。别的扩展(或本扩展)通过 `fields` 重新声明所有者字段时新增的选项,补丁不再生效,日志记一次。 - **为什么改**:#28791 之后,三处对哪些字段可补丁已一致,但选项仍不一致:合并按已合并的字段判断,安装口和 `os validate` 拒绝。 - **风险与回滚**:仓库内声明 `fieldPatches` 的包和示例为 0。补丁指向所有者已声明的选项时行为不变。回滚即还原本 PR。 - **先看**:`packages/objectql/src/registry.ts` 的 `foldExtendersOntoDefinition`(判定移到合并前)、`ownFieldsTarget`、`admittedFieldPatches`。 - **两分钟验证**:`pnpm --filter @objectstack/objectql exec vitest run src/registry-extension-field-patches.test.ts`,预期 16/16。 - **需要你做的**:无,席位安排契约复核后落地。 ## What changes - **The fold judges first, against the owner.** Per extension, before the merge, the fold asks `fieldPatchTargetMisses` of `ownFieldsTarget(owner, ownFields, carried)`: the owner's own fields, each as the OWNER declares it. That is the target the install door (`getObjectOwner(name).definition`) and `os validate` read. The merge gets only the admitted patches (`admittedFieldPatches` drops a missed field's patch, or a missed option's entry), and `reportFieldPatchMisses` names exactly the misses. One judge, one verdict, applied and reported from the same answer. - **Which fields may be patched is unchanged.** `carried` keeps #28791's rule: an own field the fold carries in record spelling, or one this body redeclares. The array-spelled pin stays green, and `applyFieldPatches` keeps its `ownFields` guard. - **No door moves.** No change to `cloud-connection` or `cli` source, no `packages/spec/**`, no new gate, error code or export. `metadata-core` changes one docblock sentence (no code). - **Docs.** One bullet in `object-extensions.mdx`: options are the owner's too, plus the residual case. **Residual (ruling 4), pinned as measured (head `301b4e3d`).** An earlier layer (an extension's `fields` redeclaration, or a tenant overlay base) leaves out an option the owner declares, and a later patch names it. All three readers admit it; `applyFieldPatches` finds no such option, so nothing of it applies. The fold then asks the same judge of the merged fields, which only names, so the patch is named once, as on main. No new rule. **Clause-② measured: yes (narrowing).** On main the fold applied a patch of an option the owner does not declare whenever it was already on the merged field: one a redeclaring extension folded first adds (served `Banking`), or one a tenant overlay base adds (served `Bulk`). Now neither applies, and each is named once; both doors already refused it. ## Tests **Round 2 (head `301b4e3d`, contract review should-fixes S1/S2).** 17 tests in the file (6 new). Red on main `663b7c08`: 3 failed | 14 passed (17): the two redeclare cases and the tenant overlay case (wholesale served Bulk). Second-pass ablation (`reportFieldPatchMisses` gets misses only): 2 failed | 15 passed (both residual cases); restore ok, blob == HEAD. Two-minute check: expect 17/17. objectql typecheck in full exit 0 (ledger unchanged 39/233/64); eslint exit 0; objectql full suite 520 files, 9996 tests passed. <details><summary>Red first, ablation, agreement</summary> `registry-extension-field-patches.test.ts`, 5 new cases: - another extension adds `finance` by redeclaring `industry` (priority 100), and a patcher (200) patches it; - the same extension redeclares and patches; - control: patcher folded first; - control: owner options `tech` and `retail` still apply onto the redeclared field; - the residual. Each asks the door's judge with `getObjectOwner` beside the fold. Red first: `registry.ts` restored to main `663b7c08` gave `Tests 3 failed | 13 passed (16)`. Both redeclare cases served `Banking`, and the residual named one line. Restore: `git checkout HEAD`, porcelain empty. Ablation (`ablation-replace.mjs`): the judge's target set back to the merged field, which gave the same 3 red. Restore: `ok restored: blob == HEAD`, `git diff HEAD` empty, marker count 0. Agreement cases (green on main by construction, since the doors already read the owner): - install-local 2e refuses `finance` with 422 beside an installed takeover; - `findFieldPatchTargetRefusals` refuses it in both the same and another extension. </details> <details><summary>Suites, typecheck, lint, gates, dogfood</summary> Head `23083aa1`, after a whole-repo `pnpm build` (exit 0), each under the verify lock: - objectql: 520 files, 9995 tests; - metadata-core: 22 files, 885 tests; - cloud-connection: 67 files, 813 tests; - cli unit: 368 files, 5195 tests. All passed. `tsc --noEmit` exits 0 on all four, and `eslint --no-inline-config` exits 0 on the 5 touched files. Gates: `dispatch-gates.mjs --commands` derived 95; all 95 ran, every exit 0. `--ran`: 95 derived, 95 run, 0 NOT-MEASURED. Dogfood battery (narrowing arm): `pnpm --filter @objectstack/dogfood test`, VERDICT command-exit 0. Test Files 380 passed, 3 skipped; Tests 3114 passed, 28 skipped. </details> ## Acceptance notes - `origin/main` has moved 10 commits since the gates; `git merge-tree` against it is clean. Seat 1's PR #28811 has not landed; whichever lands second merges main once. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01FEkQ5oTRCUNQ4EA3YV84sZ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 3e62a22 commit 4a49ad7

7 files changed

Lines changed: 314 additions & 31 deletions

File tree

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
---
2+
'@objectstack/objectql': minor
3+
---
4+
5+
fix(objectql)!: the registry fold judges an option patch against the options the object's owner declares, as the install door and `os validate` do (#28809)
6+
7+
Clause-②: yes (narrowing) — the object fold every read serves no longer applies a `fieldPatches` option patch whose option the owner's field does not declare: one an extension's `fields` redeclaration adds (applied on `main` when the redeclaring extension folded first), or one a tenant overlay base adds (applied on `main`). Nothing is widened, and no door's accept set moves.
8+
Metadata-protocol: narrowed
9+
10+
<!-- adr-0087: not-required (no-migration-prescription) No key of any schema is added, removed, renamed or re-shaped, and no export is added or removed: the install door and `os validate` already refuse this patch, so a package that installs today never carries it. No package or example in this repository declares `fieldPatches`, so there is nothing for a migration to rewrite. -->
11+
12+
**BREAKING** narrowing in the object fold every read serves (`@objectstack/objectql`).
13+
14+
- **Before.** An extension can take an owner field over through `fields` and add an option the owner does not declare, and a tenant overlay base can add one too. A `fieldPatches` option patch of such an added option was judged by the fold against the field as merged so far, so it applied whenever the added option was already there: a tenant overlay base's option, or a redeclaring extension that folded first (a lower `priority`, or the same extension). The install door and `os validate` judge the owner's own declaration and refused the same patch.
15+
- **Now.** The fold asks the one judge (`fieldPatchTargetMisses`, `@objectstack/metadata-core`) of the owner's fields as the owner declares them, and applies only what it admits. A patch of an option the owner does not declare is not applied and is named once in the log, whatever the priorities, as any other unmatched patch is. Which fields a patch reaches is unchanged: the owner's own.
16+
- **Residual, unchanged rule.** When an earlier layer leaves out an option the owner declares (an extension's `fields` redeclaration, or a tenant overlay base), a patch of that option is admitted at all three readers, and the fold has no such option to relabel, so nothing of it applies. The fold asks the same judge once more of the merged fields, which only names: the patch is named once, as on `main`. Nothing is dropped or refused by that second asking.
17+
- **Who sees it.** A package whose extension patches an option the owner does not declare, which only an extension's `fields` redeclaration or a tenant overlay base adds. Both doors already refused such a package. Measured in this repository: none (`fieldPatches` appears in no package or example outside the implementing code and tests).
18+
19+
**The remedy.** Set that option's label where the option is declared: in the `fields` entry of the extension that redeclares the field, or in the tenant's own customization of the field.

‎content/docs/data-modeling/object-extensions.mdx‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -141,6 +141,12 @@ reason.
141141
`os validate` when the target is in the same stack. A field whose options come from a
142142
shared picklist has no option labels of its own to patch. At boot, an unmatched patch
143143
is not applied and is named once in the log; the boot carries on.
144+
- **Options are the owner's too.** A patch is judged against the options the owner
145+
declares, everywhere: an option another extension (or your own) adds by replacing the
146+
field through `fields`, or one a tenant's customization adds, is not one a patch can
147+
name, so the install door and `os validate` refuse it and at boot it is not applied
148+
and is named once. If an earlier layer leaves out an option the owner declares, a patch
149+
of that option is admitted but has nothing to relabel, and the boot names it once.
144150

145151
---
146152

‎packages/cli/src/utils/field-patch-targets.test.ts‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,22 @@ describe('findFieldPatchTargetRefusals', () => {
3535
})).toEqual([]);
3636
});
3737

38+
it('[#28809] judges an option against the declared object, never against an extension that redeclares the field through `fields`', () => {
39+
const takeover = { industry: { ...account.fields.industry, options: [...account.fields.industry.options, { value: 'finance', label: 'Finance' }] } };
40+
const refusals = findFieldPatchTargetRefusals({
41+
objects: [account],
42+
objectExtensions: [
43+
{ extend: 'fpt_account', fields: takeover },
44+
{ extend: 'fpt_account', fields: takeover, fieldPatches: { industry: { options: { finance: { label: 'Banking' } } } } },
45+
{ extend: 'fpt_account', fieldPatches: { industry: { options: { finance: { label: 'Banking' } } } } },
46+
],
47+
});
48+
expect(refusals.map((r) => r.path)).toEqual([
49+
'objectExtensions[1].fieldPatches.industry.options.finance',
50+
'objectExtensions[2].fieldPatches.industry.options.finance',
51+
]);
52+
});
53+
3854
it('leaves a target outside the composed stack to the install door', () => {
3955
expect(findFieldPatchTargetRefusals({
4056
objectExtensions: [{ extend: 'crm_account', fieldPatches: { anything: { label: 'X' } } }],

‎packages/cloud-connection/src/marketplace-install-local-field-patch-targets.test.ts‎

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,10 +75,18 @@ function makeC(body: any) {
7575
* A real registry holding the installed vendor object, and — when `region` is
7676
* set — an installed extension that adds `crm_account.region`.
7777
*/
78-
function installedObjects({ region = false } = {}): SchemaRegistry {
78+
function installedObjects({ region = false, takeover = false } = {}): SchemaRegistry {
7979
const reg = new SchemaRegistry({ multiTenant: false, searchCompanion: false } as never);
8080
reg.logLevel = 'silent';
8181
reg.registerObject(structuredClone(ACCOUNT) as any, BASE, undefined, 'own');
82+
if (takeover) {
83+
// [#28809] An installed extension that takes `industry` over through
84+
// `fields`, adding the option `finance` the owner does not declare.
85+
reg.registerObject(
86+
{ name: ACCOUNT.name, fields: { industry: { ...ACCOUNT.fields.industry, options: [...ACCOUNT.fields.industry.options, { value: 'finance', label: 'Finance' }] } } } as any,
87+
'com.example.crm-industry', undefined, 'extend', 100,
88+
);
89+
}
8290
if (region) {
8391
reg.registerObject(
8492
{ name: ACCOUNT.name, fields: { region: { type: 'text', label: 'Region' } } } as any,
@@ -192,6 +200,17 @@ describe('install-local 2e — the field-patch target check (ADR-0146 D10)', ()
192200
expect(h.syncs()).toBe(0);
193201
});
194202

203+
it('[#28809] refuses a patch of an option only another installed extension adds by redeclaring the owner field, as the fold leaves it unapplied', async () => {
204+
const objects = installedObjects({ takeover: true });
205+
// The resolved object does carry the option; the owner does not declare it.
206+
expect((objects.getObject(ACCOUNT.name) as any).fields.industry.options.map((o: any) => o.value)).toContain('finance');
207+
const h = await mount(dir, objects);
208+
const res = await h.install({ manifest: extensionPackage({ industry: { options: { finance: { label: 'Banking' } } } }) });
209+
expectRefusal(res, ['objectExtensions[0].fieldPatches.industry.options.finance']);
210+
expect(res.payload.error.details.refusals[0]).toMatchObject({ object: 'crm_account', field: 'industry', option: 'finance', reason: 'option' });
211+
expect(h.installRegistrations()).toEqual([]);
212+
});
213+
195214
it('…while a patch of the owner\'s own field installs beside that extension', async () => {
196215
const h = await mount(dir, installedObjects({ region: true }));
197216
const res = await h.install({ manifest: extensionPackage({ industry: { label: 'Sector' } }) });

‎packages/metadata-core/src/extension-field-patch-targets.ts‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -84,10 +84,11 @@ function optionValuesOf(field: Record<string, unknown>): string[] {
8484
* matched.
8585
*
8686
* `target` is the extended object's OWN declaration as the reader sees it —
87-
* a patch reaches the owner's own fields only, never one an extension adds:
88-
* the registry's owner declaration at the install door, the owner's fields as
89-
* the fold has merged them in the fold, the composed stack's declaration at
90-
* `os validate`.
87+
* a patch reaches the owner's own fields only, never one an extension adds,
88+
* and an option the owner's field declares, never one an extension adds by
89+
* redeclaring the field through `fields` (#28809): the registry's owner
90+
* declaration at the install door and in the fold, the composed stack's
91+
* declaration at `os validate`.
9192
*/
9293
export function fieldPatchTargetMisses(object: string, fieldPatches: unknown, target: unknown): FieldPatchTargetMiss[] {
9394
if (!isRecord(fieldPatches)) return [];

‎packages/objectql/src/registry-extension-field-patches.test.ts‎

Lines changed: 145 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
import { describe, it, expect, vi, afterEach } from 'vitest';
2121
import { SchemaRegistry } from './registry.js';
2222
import { ObjectQL } from './engine.js';
23+
import { fieldPatchTargetRefusals } from '@objectstack/metadata-core';
2324

2425
const OWNER_PKG = 'com.vendor.crm';
2526
const EXT_PKG = 'com.acme.crm-display';
@@ -243,6 +244,150 @@ describe('ADR-0146 D10 — fieldPatches is applied by mergeObjectDefinitions on
243244
});
244245
});
245246

247+
describe('#28809 — an option patch is judged against the owner\'s declared options, at the fold as at the doors', () => {
248+
const TAKEOVER_PKG = 'com.acme.industry-takeover';
249+
250+
/**
251+
* `crm_account.industry` taken over through `fields`: the owner's two
252+
* options, the second relabelled, plus `finance`, which the owner does not
253+
* declare — or, with `dropRetail`, the owner's `tech` alone.
254+
*/
255+
const takenOverIndustry = ({ dropRetail = false } = {}) => ({
256+
...vendorAccount().fields.industry,
257+
options: dropRetail
258+
? [{ value: 'tech', label: 'Tech' }]
259+
: [
260+
{ value: 'tech', label: 'Tech' },
261+
{ value: 'retail', label: 'Retail stores' },
262+
{ value: 'finance', label: 'Finance' },
263+
],
264+
});
265+
266+
function takeOver(reg: SchemaRegistry, priority: number, opts: { dropRetail?: boolean } = {}): void {
267+
reg.registerObject(
268+
{ name: OBJ, fields: { industry: takenOverIndustry(opts) } } as any,
269+
TAKEOVER_PKG, undefined, 'extend', priority,
270+
);
271+
}
272+
273+
/**
274+
* What the install door (`getObjectOwner(name).definition`) and `os validate`
275+
* (the composed stack's declared object) both ask: the one judge, against the
276+
* owner's declaration. The fold must answer the same.
277+
*/
278+
const doorRefusals = (reg: SchemaRegistry, objectExtensions: unknown[]) =>
279+
fieldPatchTargetRefusals(objectExtensions, (name) => reg.getObjectOwner(name)?.definition).map((r) => r.path);
280+
281+
const optionLabel = (body: any, value: string) =>
282+
industryOf(body).options.find((o: any) => o.value === value)?.label;
283+
284+
const patchLines = (warn: { mock: { calls: unknown[][] } }, needle: string): string[] =>
285+
warn.mock.calls.map((c) => String(c[0])).filter((l) => l.includes(needle));
286+
287+
it('another extension adds an option by redeclaring the owner field: a patch of that option is not applied, and is named once — as the door and os validate refuse it', () => {
288+
const warn = vi.spyOn(console, 'warn').mockImplementation(() => {});
289+
const reg = withVendor();
290+
takeOver(reg, 100);
291+
const patch = { industry: { options: { finance: { label: 'Banking' } } } };
292+
extend(reg, patch, EXT_PKG, 200);
293+
294+
expect(doorRefusals(reg, [{ extend: OBJ, fieldPatches: patch }]))
295+
.toEqual(['objectExtensions[0].fieldPatches.industry.options.finance']);
296+
const served = reg.getObject(OBJ) as any;
297+
// The takeover itself still serves: the option exists, with its own label.
298+
expect(optionLabel(served, 'finance')).toBe('Finance');
299+
reg.invalidateAll();
300+
reg.getObject(OBJ);
301+
const lines = patchLines(warn, 'fieldPatches.industry.options.finance');
302+
expect(lines).toHaveLength(1);
303+
expect(lines[0]).toContain(EXT_PKG);
304+
});
305+
306+
it('…and the same extension redeclaring the field and patching its added option is not applied either, and is named once', () => {
307+
const warn = vi.spyOn(console, 'warn').mockImplementation(() => {});
308+
const reg = withVendor();
309+
const entry = {
310+
fields: { industry: takenOverIndustry() },
311+
fieldPatches: { industry: { options: { finance: { label: 'Banking' } } } },
312+
};
313+
reg.registerObject({ name: OBJ, ...entry } as any, EXT_PKG, undefined, 'extend', 200);
314+
315+
expect(doorRefusals(reg, [{ extend: OBJ, ...entry }]))
316+
.toEqual(['objectExtensions[0].fieldPatches.industry.options.finance']);
317+
expect(optionLabel(reg.getObject(OBJ), 'finance')).toBe('Finance');
318+
expect(patchLines(warn, 'fieldPatches.industry.options.finance')).toHaveLength(1);
319+
});
320+
321+
it('control: the patcher folded BEFORE the redeclaring extension (lower priority) — not applied and named, as on main', () => {
322+
const warn = vi.spyOn(console, 'warn').mockImplementation(() => {});
323+
const reg = withVendor();
324+
takeOver(reg, 200);
325+
extend(reg, { industry: { options: { finance: { label: 'Banking' } } } }, EXT_PKG, 100);
326+
327+
expect(optionLabel(reg.getObject(OBJ), 'finance')).toBe('Finance');
328+
expect(patchLines(warn, 'fieldPatches.industry.options.finance')).toHaveLength(1);
329+
});
330+
331+
it('control: a patch of an option the owner declares still applies onto the redeclared field, and nothing is named', () => {
332+
const warn = vi.spyOn(console, 'warn').mockImplementation(() => {});
333+
const reg = withVendor();
334+
takeOver(reg, 100);
335+
const patch = { industry: { label: 'Sector', options: { tech: { label: 'Technology' }, retail: { label: 'Shops' } } } };
336+
extend(reg, patch, EXT_PKG, 200);
337+
338+
expect(doorRefusals(reg, [{ extend: OBJ, fieldPatches: patch }])).toEqual([]);
339+
const served = reg.getObject(OBJ) as any;
340+
expect(industryOf(served).label).toBe('Sector');
341+
expect(optionLabel(served, 'tech')).toBe('Technology');
342+
expect(optionLabel(served, 'retail')).toBe('Shops');
343+
expect(optionLabel(served, 'finance')).toBe('Finance');
344+
expect(patchLines(warn, 'fieldPatches')).toEqual([]);
345+
});
346+
347+
it('residual: an owner option an earlier extension\'s redeclaration left out — the judge admits the patch, the fold has no such option to relabel, nothing of it is applied, and it is named once', () => {
348+
const warn = vi.spyOn(console, 'warn').mockImplementation(() => {});
349+
const reg = withVendor();
350+
takeOver(reg, 100, { dropRetail: true });
351+
const patch = { industry: { label: 'Sector', options: { retail: { label: 'Shops' } } } };
352+
extend(reg, patch, EXT_PKG, 200);
353+
354+
// The door and os validate admit it: `retail` is in the owner's declaration.
355+
expect(doorRefusals(reg, [{ extend: OBJ, fieldPatches: patch }])).toEqual([]);
356+
const served = reg.getObject(OBJ) as any;
357+
// The takeover's options are served as the takeover declares them; the
358+
// patch's field key still applies.
359+
expect(industryOf(served).options).toEqual([{ value: 'tech', label: 'Tech' }]);
360+
expect(industryOf(served).label).toBe('Sector');
361+
reg.invalidateAll();
362+
reg.getObject(OBJ);
363+
// Named once (what the fold skipped), by the same judge asked of the merged field.
364+
const lines = patchLines(warn, 'fieldPatches');
365+
expect(lines).toHaveLength(1);
366+
expect(lines[0]).toContain('fieldPatches.industry.options.retail');
367+
expect(lines[0]).toContain(EXT_PKG);
368+
});
369+
370+
it('residual: a tenant overlay base that leaves an owner option out — admitted, nothing to relabel, named once; an option the tenant adds is not patched', () => {
371+
const warn = vi.spyOn(console, 'warn').mockImplementation(() => {});
372+
const reg = withVendor();
373+
const patch = { industry: { options: { retail: { label: 'Shops' }, wholesale: { label: 'Bulk' } } } };
374+
extend(reg, patch, EXT_PKG, 200);
375+
const overlay = vendorAccount();
376+
overlay.fields.industry.options = [{ value: 'tech', label: 'Tech' }, { value: 'wholesale', label: 'Wholesale' }];
377+
reg.registerObject({ ...overlay, _provenance: 'org' }, 'sys_metadata', undefined, 'overlay');
378+
379+
// The doors read the owner: `retail` admitted, `wholesale` (the tenant's) refused.
380+
expect(doorRefusals(reg, [{ extend: OBJ, fieldPatches: patch }]))
381+
.toEqual(['objectExtensions[0].fieldPatches.industry.options.wholesale']);
382+
const served = reg.getObject(OBJ) as any;
383+
expect(industryOf(served).options).toEqual([{ value: 'tech', label: 'Tech' }, { value: 'wholesale', label: 'Wholesale' }]);
384+
reg.invalidateAll();
385+
reg.getObject(OBJ);
386+
expect(patchLines(warn, 'fieldPatches.industry.options.retail')).toHaveLength(1);
387+
expect(patchLines(warn, 'fieldPatches.industry.options.wholesale')).toHaveLength(1);
388+
});
389+
});
390+
246391
describe('ADR-0146 D10 — the manifest loop carries fieldPatches onto the extension def', () => {
247392
it('a package\'s objectExtensions entry patches the target through ObjectQL.registerApp', () => {
248393
const ql = new ObjectQL();

0 commit comments

Comments
 (0)