diff --git a/.changeset/28858-extension-legacy-key-fold.md b/.changeset/28858-extension-legacy-key-fold.md new file mode 100644 index 00000000000..dc3156ce44e --- /dev/null +++ b/.changeset/28858-extension-legacy-key-fold.md @@ -0,0 +1,14 @@ +--- +'@objectstack/objectql': patch +--- + +fix(objectql): an object extension whose `extend` folds to a legacy `ns__name` key is filed under that key, and never opens a second, owner-less object entry (#28858) + +Clause-②: no — no accept set moves and nothing is exported; an extension that already registered now lands on the object it names. Measured reach: no package or example in this repository or in hotcrm spells an `extend` that folds. +Metadata-protocol: none + +`getObject` reads a short name as a legacy `__` key when that is the only object it can mean (an object a package still ships as `crm__case`, which `ObjectSchema` accepts). The registry used to file an extension under the name it is written with. So a manifest `objectExtensions` entry or a stored `object_extension` row with `extend: 'case'` opened a second entry, `case`, with no owner, beside `crm__case`. Its fields reached no read of the object, and nothing checked the entry. + +- **Now.** Every entry that registers an extension (the manifest loop and the stored row's hydration, both through `registerObject`) files it under the key `getObject` reads its `extend` as. The fields land on `crm__case`, and no `case` entry opens. The spelling is named once per spelling and package at `warn`, with the key to write. +- **Never a boot refusal.** The manifest loop and the hydration run while packages are still registering. The `object_extension` item door keeps refusing the folded spelling at save (#28834). +- **Unchanged.** An `extend` spelled with the registered key, or naming an object registered under that exact name, registers as before, with no line. `computeFQN` and `parseFQN` are untouched. A legacy owner that registers after the extension is not yet there to fold to, so that extension is filed as written, as before. diff --git a/packages/objectql/src/registry-extension-legacy-key-fold.test.ts b/packages/objectql/src/registry-extension-legacy-key-fold.test.ts new file mode 100644 index 00000000000..65d0294655b --- /dev/null +++ b/packages/objectql/src/registry-extension-legacy-key-fold.test.ts @@ -0,0 +1,112 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#28858, ADR-0146 D10] An `extend` that `getObject` folds to a legacy + * `__` key never opens a second object entry. + * + * A vendor package still ships its Case under the legacy key `crm__case` + * (the published `ObjectSchema` accepts the name), so `getObject('case')` + * folds to it. A second package extends it as `extend: 'case'`. The + * extension is filed under the key the object is registered under, its field + * lands on the vendor's object, no owner-less `case` entry opens beside it, + * and the spelling is named once at `warn`. Never a boot refusal (AGENTS.md + * *Startup registry reads*). The `object_extension` item door still refuses + * the folded spelling at save (#28853, pinned in `@objectstack/runtime`). + */ + +import { describe, it, expect, vi, afterEach } from 'vitest'; +import { SchemaRegistry } from './registry.js'; +import { ObjectQL } from './engine.js'; + +const VENDOR = 'com.vendor.crm'; +const EXT = 'com.acme.case-region'; +const LEGACY_KEY = 'crm__case'; +const SHORT = 'case'; + +const vendorCase = () => ({ + name: LEGACY_KEY, + label: 'Case', + fields: { subject: { name: 'subject', type: 'text', label: 'Subject' } }, +}); + +const region = { region: { name: 'region', type: 'text', label: 'Region' } }; + +function registry(): SchemaRegistry { + const reg = new SchemaRegistry({ multiTenant: false, searchCompanion: false } as never); + reg.logLevel = 'silent'; + return reg; +} + +const spellingLines = (warn: { mock: { calls: unknown[][] } }): string[] => + warn.mock.calls.map((c) => String(c[0])).filter((l) => l.includes(`'${LEGACY_KEY}'`) && l.includes(EXT)); + +afterEach(() => vi.restoreAllMocks()); + +describe('#28858 — an extension whose `extend` folds to a legacy key lands on the object it folds to', () => { + it('a manifest objectExtensions entry: the field is on the vendor\'s object, no second entry opens, named once', () => { + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); + const ql = new ObjectQL(); + (ql.registry as SchemaRegistry).logLevel = 'silent'; + ql.registerApp({ id: VENDOR, name: 'CRM', version: '1.0.0', objects: [vendorCase()] }); + ql.registerApp({ + id: EXT, name: 'Case region', version: '1.0.0', + objectExtensions: [{ extend: SHORT, fields: region }], + }); + + const reg = ql.registry as SchemaRegistry; + expect(Object.keys((reg.getObject(LEGACY_KEY) as any).fields)).toEqual(expect.arrayContaining(['subject', 'region'])); + expect(reg.getObjectContributors(SHORT)).toEqual([]); + expect(reg.getObjectContributors(LEGACY_KEY).map((c) => [c.packageId, c.ownership])) + .toEqual([[VENDOR, 'own'], [EXT, 'extend']]); + expect(reg.getAllObjects().map((o) => o.name).filter((n) => n === SHORT || n === LEGACY_KEY)).toEqual([LEGACY_KEY]); + // The short name still reads the one object, with the field. + expect(Object.keys((reg.getObject(SHORT) as any).fields)).toContain('region'); + + // Re-reading and re-registering name it no more than once. + reg.invalidateAll(); + reg.getObject(LEGACY_KEY); + ql.registerApp({ id: EXT, name: 'Case region', version: '1.0.0', objectExtensions: [{ extend: SHORT, fields: region }] }); + const lines = spellingLines(warn); + expect(lines).toHaveLength(1); + expect(lines[0]).toContain(`'${SHORT}'`); + }); + + it('registerObjectExtension (the stored row\'s verb) files under the resolved key and answers it', () => { + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); + const reg = registry(); + reg.registerObject(vendorCase() as any, VENDOR, undefined, 'own'); + + const filed = reg.registerObjectExtension({ extend: SHORT, fields: region, _provenance: 'org' }, EXT); + + expect(filed).toBe(LEGACY_KEY); + expect(reg.getObjectContributors(SHORT)).toEqual([]); + expect(Object.keys((reg.getObject(LEGACY_KEY) as any).fields)).toContain('region'); + expect(spellingLines(warn)).toHaveLength(1); + // The withdrawal the row delete makes reaches the same entry. + expect(reg.withdrawObjectExtension(SHORT, EXT)).toBe(true); + expect(Object.keys((reg.getObject(LEGACY_KEY) as any).fields)).not.toContain('region'); + }); + + it('control: an extension spelled with the registered key registers as before, with no warn', () => { + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); + const reg = registry(); + reg.registerObject(vendorCase() as any, VENDOR, undefined, 'own'); + + expect(reg.registerObjectExtension({ extend: LEGACY_KEY, fields: region }, EXT)).toBe(LEGACY_KEY); + expect(Object.keys((reg.getObject(LEGACY_KEY) as any).fields)).toContain('region'); + expect(reg.getObjectContributors(SHORT)).toEqual([]); + expect(spellingLines(warn)).toEqual([]); + }); + + it('control: an object registered under the exact name keeps its extensions, even beside a legacy key that shares the short name', () => { + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); + const reg = registry(); + reg.registerObject(vendorCase() as any, VENDOR, undefined, 'own'); + reg.registerObject({ name: SHORT, label: 'Support case', fields: { title: { name: 'title', type: 'text', label: 'Title' } } } as any, 'com.other.support', undefined, 'own'); + + expect(reg.registerObjectExtension({ extend: SHORT, fields: region }, EXT)).toBe(SHORT); + expect(reg.getObjectContributors(SHORT).map((c) => c.ownership)).toEqual(['own', 'extend']); + expect(Object.keys((reg.getObject(LEGACY_KEY) as any).fields)).not.toContain('region'); + expect(spellingLines(warn)).toEqual([]); + }); +}); diff --git a/packages/objectql/src/registry.ts b/packages/objectql/src/registry.ts index 8827e579eb5..9761f56f108 100644 --- a/packages/objectql/src/registry.ts +++ b/packages/objectql/src/registry.ts @@ -4285,6 +4285,12 @@ export class SchemaRegistry { if (ownership === 'extend') { const extendRefusal = extendContributionRefusal(schema, packageId); if (extendRefusal) throw extendRefusal; + // [#28858] …filed under the key the object it extends is registered + // under — see {@link extensionTargetKey}. Every entry that registers an + // extension reaches this line: the manifest `objectExtensions` loop and + // a stored `object_extension` row ({@link registerObjectExtension}). + const target = this.extensionTargetKey(String(schema.name ?? ''), packageId); + if (target !== schema.name) schema = { ...schema, name: target }; } // [ADR-0144 D6] A `summary` roll-up that reads a `record`-mode amount — // `sum` / `avg` / `min` / `max` of a child field whose rows each carry @@ -6183,6 +6189,46 @@ export class SchemaRegistry { ); } + /** + * [#28858] Spellings of an `extend` already named, keyed + * `spelled|resolved|package`. Per registry, for the reason + * {@link shadowedExtensionQueueWarned} gives. + */ + private extensionSpellingWarned = new Set(); + + /** + * [#28858, ADR-0146 D10] The key an extension of `name` is filed under: the + * key {@link getObject} reads `name` as. That is `name` itself whenever an + * entry is registered under it, or nothing is registered that `name` folds + * to. It differs only for a short name `getObject` folds to a legacy + * `__` key ({@link resolveObjectKey}); filing such an extension + * under the short name would open a second, owner-less entry beside the + * object it meant, which nothing serves and nothing checks. + * + * The folded spelling is named once at `warn`, with the key to write. ⛔ + * Never a refusal: the manifest loop and the boot hydration run while + * packages are still registering (AGENTS.md *Startup registry reads*). The + * `object_extension` item door refuses the same spelling at save (#28834), + * where the author is present to fix it. A legacy owner that registers + * AFTER the extension is not yet there to fold to, so that extension is + * filed under its own spelling, as before. + */ + private extensionTargetKey(name: string, packageId: string | undefined): string { + if (name === '' || this.objectContributors.has(name)) return name; + const resolved = this.resolveObjectKey(name); + if (resolved === undefined || resolved === name) return name; + const key = `${name}|${resolved}|${packageId ?? ''}`; + if (!this.extensionSpellingWarned.has(key)) { + this.extensionSpellingWarned.add(key); + console.warn( + `[Registry] An extension${packageId ? ` from package "${packageId}"` : ''} names its object '${name}', ` + + `which is registered as '${resolved}': the extension is filed under '${resolved}', the object it ` + + `reads as. Write \`extend: '${resolved}'\` to name the object by the key it is registered under.`, + ); + } + return resolved; + } + /** * [#28574] Withdraw ONE package's `'extend'` contribution to an object — the * delete of that package's `object_extension` item. The owner, the overlay diff --git a/packages/runtime/src/domains/meta-object-extension-item-door-fit.test.ts b/packages/runtime/src/domains/meta-object-extension-item-door-fit.test.ts index cf3f0e3b560..ea91611ad35 100644 --- a/packages/runtime/src/domains/meta-object-extension-item-door-fit.test.ts +++ b/packages/runtime/src/domains/meta-object-extension-item-door-fit.test.ts @@ -29,7 +29,7 @@ * sentence and that no row was left. */ -import { describe, it, expect, beforeAll, afterAll, afterEach } from 'vitest'; +import { describe, it, expect, beforeAll, afterAll, afterEach, vi } from 'vitest'; import { mkdtempSync, rmSync } from 'node:fs'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; @@ -199,3 +199,41 @@ describe('an object_extension item names its target by the key it is registered expect(engine.registry.getObjectContributors(LEGACY_SHORT)).toEqual([]); }); }); + +describe('a stored object_extension row whose extend folds to a legacy key lands on that object at the boot hydration (#28858)', () => { + it('the field is on the legacy-keyed object, no second entry opens, and the spelling is named once', async () => { + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); + try { + const { engine, p } = await boot(); + // A row stored under the short spelling: the item door refuses + // that spelling now (#28834), so it is a row written before it. + // Stored as the registered key, then respelled at rest. + await p.saveMetaItem({ type: 'object_extension', name: LEGACY_KEY, packageId: PAIR, item: region(LEGACY_KEY), mode: 'draft' }); + await p.publishMetaItem({ type: 'object_extension', name: LEGACY_KEY, packageId: PAIR }); + const stored: any[] = ((await engine.find('sys_metadata', { where: { type: 'object_extension' }, context: { isSystem: true } })) ?? []) + .filter((r: any) => r.package_id === PAIR); + expect(stored).toHaveLength(1); + const body = typeof stored[0].metadata === 'string' ? JSON.parse(stored[0].metadata) : stored[0].metadata; + await engine.update('sys_metadata', { + id: stored[0].id, + name: LEGACY_SHORT, + metadata: JSON.stringify({ ...body, extend: LEGACY_SHORT }), + }, { context: { isSystem: true } }); + // A fresh process: the registry holds no contribution of the pair. + engine.registry.withdrawObjectExtension(LEGACY_KEY, PAIR); + expect(engine.registry.getObjectContributors(LEGACY_KEY).map((c: any) => c.packageId)).toEqual([BASE]); + + const result = await p.loadMetaFromDb(); + expect(result.errors).toBe(0); + expect(engine.registry.getObjectContributors(LEGACY_SHORT)).toEqual([]); + expect(engine.registry.getObjectContributors(LEGACY_KEY).map((c: any) => [c.packageId, c.ownership])) + .toEqual([[BASE, 'own'], [PAIR, 'extend']]); + expect(Object.keys((engine.registry.getObject(LEGACY_KEY) as any)?.fields ?? {})).toEqual(expect.arrayContaining(['region__c', 'subject'])); + const lines = warn.mock.calls.map((c) => String(c[0])) + .filter((l) => l.includes(`'${LEGACY_SHORT}'`) && l.includes(`'${LEGACY_KEY}'`) && l.includes(PAIR)); + expect(lines).toHaveLength(1); + } finally { + warn.mockRestore(); + } + }); +});