fix(objectql)!: the registry fold judges an option patch against the options the owner declares, as the install door and os validate do (#28809)

.changeset/28809-field-patch-owner-declared-options.md

@@ -0,0 +1,19 @@
+---
+'@objectstack/objectql': minor
+---
+
+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)
+
+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.
+Metadata-protocol: narrowed
+
+<!-- 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. -->
+
+**BREAKING** narrowing in the object fold every read serves (`@objectstack/objectql`).
+
+- **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.
+- **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.
+- **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.
+- **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).
+
+**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

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

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

@@ -35,6 +35,22 @@ describe('findFieldPatchTargetRefusals', () => {
     })).toEqual([]);
   });
 
+  it('[#28809] judges an option against the declared object, never against an extension that redeclares the field through `fields`', () => {
+    const takeover = { industry: { ...account.fields.industry, options: [...account.fields.industry.options, { value: 'finance', label: 'Finance' }] } };
+    const refusals = findFieldPatchTargetRefusals({
+      objects: [account],
+      objectExtensions: [
+        { extend: 'fpt_account', fields: takeover },
+        { extend: 'fpt_account', fields: takeover, fieldPatches: { industry: { options: { finance: { label: 'Banking' } } } } },
+        { extend: 'fpt_account', fieldPatches: { industry: { options: { finance: { label: 'Banking' } } } } },
+      ],
+    });
+    expect(refusals.map((r) => r.path)).toEqual([
+      'objectExtensions[1].fieldPatches.industry.options.finance',
+      'objectExtensions[2].fieldPatches.industry.options.finance',
+    ]);
+  });
+
   it('leaves a target outside the composed stack to the install door', () => {
     expect(findFieldPatchTargetRefusals({
       objectExtensions: [{ extend: 'crm_account', fieldPatches: { anything: { label: 'X' } } }],

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

@@ -75,10 +75,18 @@ function makeC(body: any) {
  * A real registry holding the installed vendor object, and — when `region` is
  * set — an installed extension that adds `crm_account.region`.
  */
-function installedObjects({ region = false } = {}): SchemaRegistry {
+function installedObjects({ region = false, takeover = false } = {}): SchemaRegistry {
     const reg = new SchemaRegistry({ multiTenant: false, searchCompanion: false } as never);
     reg.logLevel = 'silent';
     reg.registerObject(structuredClone(ACCOUNT) as any, BASE, undefined, 'own');
+    if (takeover) {
+        // [#28809] An installed extension that takes `industry` over through
+        // `fields`, adding the option `finance` the owner does not declare.
+        reg.registerObject(
+            { name: ACCOUNT.name, fields: { industry: { ...ACCOUNT.fields.industry, options: [...ACCOUNT.fields.industry.options, { value: 'finance', label: 'Finance' }] } } } as any,
+            'com.example.crm-industry', undefined, 'extend', 100,
+        );
+    }
     if (region) {
         reg.registerObject(
             { 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)', ()
         expect(h.syncs()).toBe(0);
     });
 
+    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 () => {
+        const objects = installedObjects({ takeover: true });
+        // The resolved object does carry the option; the owner does not declare it.
+        expect((objects.getObject(ACCOUNT.name) as any).fields.industry.options.map((o: any) => o.value)).toContain('finance');
+        const h = await mount(dir, objects);
+        const res = await h.install({ manifest: extensionPackage({ industry: { options: { finance: { label: 'Banking' } } } }) });
+        expectRefusal(res, ['objectExtensions[0].fieldPatches.industry.options.finance']);
+        expect(res.payload.error.details.refusals[0]).toMatchObject({ object: 'crm_account', field: 'industry', option: 'finance', reason: 'option' });
+        expect(h.installRegistrations()).toEqual([]);
+    });
+
     it('…while a patch of the owner\'s own field installs beside that extension', async () => {
         const h = await mount(dir, installedObjects({ region: true }));
         const res = await h.install({ manifest: extensionPackage({ industry: { label: 'Sector' } }) });

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

@@ -84,10 +84,11 @@ function optionValuesOf(field: Record<string, unknown>): string[] {
  * matched.
  *
  * `target` is the extended object's OWN declaration as the reader sees it —
- * a patch reaches the owner's own fields only, never one an extension adds:
- * the registry's owner declaration at the install door, the owner's fields as
- * the fold has merged them in the fold, the composed stack's declaration at
- * `os validate`.
+ * a patch reaches the owner's own fields only, never one an extension adds,
+ * and an option the owner's field declares, never one an extension adds by
+ * redeclaring the field through `fields` (#28809): the registry's owner
+ * declaration at the install door and in the fold, the composed stack's
+ * declaration at `os validate`.
  */
 export function fieldPatchTargetMisses(object: string, fieldPatches: unknown, target: unknown): FieldPatchTargetMiss[] {
   if (!isRecord(fieldPatches)) return [];

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

@@ -20,6 +20,7 @@
 import { describe, it, expect, vi, afterEach } from 'vitest';
 import { SchemaRegistry } from './registry.js';
 import { ObjectQL } from './engine.js';
+import { fieldPatchTargetRefusals } from '@objectstack/metadata-core';
 
 const OWNER_PKG = 'com.vendor.crm';
 const EXT_PKG = 'com.acme.crm-display';
@@ -243,6 +244,150 @@ describe('ADR-0146 D10 — fieldPatches is applied by mergeObjectDefinitions on
   });
 });
 
+describe('#28809 — an option patch is judged against the owner\'s declared options, at the fold as at the doors', () => {
+  const TAKEOVER_PKG = 'com.acme.industry-takeover';
+
+  /**
+   * `crm_account.industry` taken over through `fields`: the owner's two
+   * options, the second relabelled, plus `finance`, which the owner does not
+   * declare — or, with `dropRetail`, the owner's `tech` alone.
+   */
+  const takenOverIndustry = ({ dropRetail = false } = {}) => ({
+    ...vendorAccount().fields.industry,
+    options: dropRetail
+      ? [{ value: 'tech', label: 'Tech' }]
+      : [
+        { value: 'tech', label: 'Tech' },
+        { value: 'retail', label: 'Retail stores' },
+        { value: 'finance', label: 'Finance' },
+      ],
+  });
+
+  function takeOver(reg: SchemaRegistry, priority: number, opts: { dropRetail?: boolean } = {}): void {
+    reg.registerObject(
+      { name: OBJ, fields: { industry: takenOverIndustry(opts) } } as any,
+      TAKEOVER_PKG, undefined, 'extend', priority,
+    );
+  }
+
+  /**
+   * What the install door (`getObjectOwner(name).definition`) and `os validate`
+   * (the composed stack's declared object) both ask: the one judge, against the
+   * owner's declaration. The fold must answer the same.
+   */
+  const doorRefusals = (reg: SchemaRegistry, objectExtensions: unknown[]) =>
+    fieldPatchTargetRefusals(objectExtensions, (name) => reg.getObjectOwner(name)?.definition).map((r) => r.path);
+
+  const optionLabel = (body: any, value: string) =>
+    industryOf(body).options.find((o: any) => o.value === value)?.label;
+
+  const patchLines = (warn: { mock: { calls: unknown[][] } }, needle: string): string[] =>
+    warn.mock.calls.map((c) => String(c[0])).filter((l) => l.includes(needle));
+
+  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', () => {
+    const warn = vi.spyOn(console, 'warn').mockImplementation(() => {});
+    const reg = withVendor();
+    takeOver(reg, 100);
+    const patch = { industry: { options: { finance: { label: 'Banking' } } } };
+    extend(reg, patch, EXT_PKG, 200);
+
+    expect(doorRefusals(reg, [{ extend: OBJ, fieldPatches: patch }]))
+      .toEqual(['objectExtensions[0].fieldPatches.industry.options.finance']);
+    const served = reg.getObject(OBJ) as any;
+    // The takeover itself still serves: the option exists, with its own label.
+    expect(optionLabel(served, 'finance')).toBe('Finance');
+    reg.invalidateAll();
+    reg.getObject(OBJ);
+    const lines = patchLines(warn, 'fieldPatches.industry.options.finance');
+    expect(lines).toHaveLength(1);
+    expect(lines[0]).toContain(EXT_PKG);
+  });
+
+  it('…and the same extension redeclaring the field and patching its added option is not applied either, and is named once', () => {
+    const warn = vi.spyOn(console, 'warn').mockImplementation(() => {});
+    const reg = withVendor();
+    const entry = {
+      fields: { industry: takenOverIndustry() },
+      fieldPatches: { industry: { options: { finance: { label: 'Banking' } } } },
+    };
+    reg.registerObject({ name: OBJ, ...entry } as any, EXT_PKG, undefined, 'extend', 200);
+
+    expect(doorRefusals(reg, [{ extend: OBJ, ...entry }]))
+      .toEqual(['objectExtensions[0].fieldPatches.industry.options.finance']);
+    expect(optionLabel(reg.getObject(OBJ), 'finance')).toBe('Finance');
+    expect(patchLines(warn, 'fieldPatches.industry.options.finance')).toHaveLength(1);
+  });
+
+  it('control: the patcher folded BEFORE the redeclaring extension (lower priority) — not applied and named, as on main', () => {
+    const warn = vi.spyOn(console, 'warn').mockImplementation(() => {});
+    const reg = withVendor();
+    takeOver(reg, 200);
+    extend(reg, { industry: { options: { finance: { label: 'Banking' } } } }, EXT_PKG, 100);
+
+    expect(optionLabel(reg.getObject(OBJ), 'finance')).toBe('Finance');
+    expect(patchLines(warn, 'fieldPatches.industry.options.finance')).toHaveLength(1);
+  });
+
+  it('control: a patch of an option the owner declares still applies onto the redeclared field, and nothing is named', () => {
+    const warn = vi.spyOn(console, 'warn').mockImplementation(() => {});
+    const reg = withVendor();
+    takeOver(reg, 100);
+    const patch = { industry: { label: 'Sector', options: { tech: { label: 'Technology' }, retail: { label: 'Shops' } } } };
+    extend(reg, patch, EXT_PKG, 200);
+
+    expect(doorRefusals(reg, [{ extend: OBJ, fieldPatches: patch }])).toEqual([]);
+    const served = reg.getObject(OBJ) as any;
+    expect(industryOf(served).label).toBe('Sector');
+    expect(optionLabel(served, 'tech')).toBe('Technology');
+    expect(optionLabel(served, 'retail')).toBe('Shops');
+    expect(optionLabel(served, 'finance')).toBe('Finance');
+    expect(patchLines(warn, 'fieldPatches')).toEqual([]);
+  });
+
+  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', () => {
+    const warn = vi.spyOn(console, 'warn').mockImplementation(() => {});
+    const reg = withVendor();
+    takeOver(reg, 100, { dropRetail: true });
+    const patch = { industry: { label: 'Sector', options: { retail: { label: 'Shops' } } } };
+    extend(reg, patch, EXT_PKG, 200);
+
+    // The door and os validate admit it: `retail` is in the owner's declaration.
+    expect(doorRefusals(reg, [{ extend: OBJ, fieldPatches: patch }])).toEqual([]);
+    const served = reg.getObject(OBJ) as any;
+    // The takeover's options are served as the takeover declares them; the
+    // patch's field key still applies.
+    expect(industryOf(served).options).toEqual([{ value: 'tech', label: 'Tech' }]);
+    expect(industryOf(served).label).toBe('Sector');
+    reg.invalidateAll();
+    reg.getObject(OBJ);
+    // Named once (what the fold skipped), by the same judge asked of the merged field.
+    const lines = patchLines(warn, 'fieldPatches');
+    expect(lines).toHaveLength(1);
+    expect(lines[0]).toContain('fieldPatches.industry.options.retail');
+    expect(lines[0]).toContain(EXT_PKG);
+  });
+
+  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', () => {
+    const warn = vi.spyOn(console, 'warn').mockImplementation(() => {});
+    const reg = withVendor();
+    const patch = { industry: { options: { retail: { label: 'Shops' }, wholesale: { label: 'Bulk' } } } };
+    extend(reg, patch, EXT_PKG, 200);
+    const overlay = vendorAccount();
+    overlay.fields.industry.options = [{ value: 'tech', label: 'Tech' }, { value: 'wholesale', label: 'Wholesale' }];
+    reg.registerObject({ ...overlay, _provenance: 'org' }, 'sys_metadata', undefined, 'overlay');
+
+    // The doors read the owner: `retail` admitted, `wholesale` (the tenant's) refused.
+    expect(doorRefusals(reg, [{ extend: OBJ, fieldPatches: patch }]))
+      .toEqual(['objectExtensions[0].fieldPatches.industry.options.wholesale']);
+    const served = reg.getObject(OBJ) as any;
+    expect(industryOf(served).options).toEqual([{ value: 'tech', label: 'Tech' }, { value: 'wholesale', label: 'Wholesale' }]);
+    reg.invalidateAll();
+    reg.getObject(OBJ);
+    expect(patchLines(warn, 'fieldPatches.industry.options.retail')).toHaveLength(1);
+    expect(patchLines(warn, 'fieldPatches.industry.options.wholesale')).toHaveLength(1);
+  });
+});
+
 describe('ADR-0146 D10 — the manifest loop carries fieldPatches onto the extension def', () => {
   it('a package\'s objectExtensions entry patches the target through ObjectQL.registerApp', () => {
     const ql = new ObjectQL();