From 461552acedd0d0070debccf845de1350d307af6b Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 1 Nov 2026 23:43:33 +0000 Subject: [PATCH] fix(metadata-protocol): an object_extension item's refusals fit the item door An object_extension body carrying `overrides` is refused with a sentence that fits it: an extension takes no override and is stored under its target's name, so the remedy is to edit the extension itself, not to create one under a new name. An object carrying `overrides` is told both places an extension is written: the object_extension item door and the manifest's objectExtensions entry. Every other type's sentence is unchanged. The save, publish and restore gate now refuses an object_extension whose `extend` is a short name getObject folds to a different, legacy `__` key. The registry files an extension under the name it is written with, so that write opened a second, phantom object. The refusal names both spellings. Co-Authored-By: Claude --- .changeset/28834-object-extension-door-fit.md | 23 ++ .../src/protocol-object-extension-item.ts | 14 +- .../src/protocol-packaged-base-refusal.ts | 17 +- ...eta-object-extension-item-door-fit.test.ts | 201 ++++++++++++++++++ 4 files changed, 253 insertions(+), 2 deletions(-) create mode 100644 .changeset/28834-object-extension-door-fit.md create mode 100644 packages/runtime/src/domains/meta-object-extension-item-door-fit.test.ts diff --git a/.changeset/28834-object-extension-door-fit.md b/.changeset/28834-object-extension-door-fit.md new file mode 100644 index 00000000000..0e0634803c7 --- /dev/null +++ b/.changeset/28834-object-extension-door-fit.md @@ -0,0 +1,23 @@ +--- +"@objectstack/metadata-protocol": minor +--- + +fix(metadata-protocol)!: an `object_extension` item's refusals fit the item door, and an extension names its target by the key the object is registered under (#28834) + +Clause-②: no (narrowing) — the save, publish and restore doors refuse an `object_extension` whose `extend` is a short name the registry folds to a different, legacy `__` key, which they admitted before. Nothing is widened. +Metadata-protocol: narrowed + + + +**BREAKING** accept-set narrowing at the `object_extension` item doors (`@objectstack/metadata-protocol`). + +- **Before.** `getObject` resolves a short name to a legacy `__` key (a key `computeFQN` no longer produces, so only an object whose name carries `__` has one). The gate checked that the target resolved, and the registry then filed the extension under the short name it was written with: a second, phantom object beside the real one. +- **Now.** The gate refuses that write with `VALIDATION_ERROR` / 400 on `extend`, and the sentence names both spellings. Save the extension under the key the object is registered as, with `extend` set to it. +- **Who sees it.** No one measured: no manifest or object in this repository or in `hotcrm` has a `__` key, the item door itself landed one change earlier (#28574), and the boot hydration does not run this gate, so a stored row of that spelling is not refused at rest. + +Two refusal sentences change wording. Their rule, code and status stay (`NOT_OVERRIDABLE` / 403): + +- An `object_extension` carrying `overrides` is told an extension takes no override and is stored under the name of the object it extends, so the fix is to edit the extension itself. It is no longer told to create one "under a new name". +- An `object` carrying `overrides` is told both places an extension is written: an `object_extension` item saved through `/api/v1/meta/object_extension`, or an `objectExtensions` entry of the pair's manifest. + +Every other type's override sentence is unchanged. diff --git a/packages/metadata-protocol/src/protocol-object-extension-item.ts b/packages/metadata-protocol/src/protocol-object-extension-item.ts index 796dce22cb1..ac9becd8319 100644 --- a/packages/metadata-protocol/src/protocol-object-extension-item.ts +++ b/packages/metadata-protocol/src/protocol-object-extension-item.ts @@ -138,7 +138,11 @@ export interface ObjectExtensionItemSubject { * would refuse the key with the generic unknown-key sentence, so the * author reads the reason a sync would otherwise throw at registration. * 3. The row name is the target: `extend` equals the name it is filed under. - * 4. The target object is registered. + * 4. The target object is registered, under the very key `extend` spells + * (#28834): `getObject` also folds a short name to a legacy + * `__` key, while the registry files an extension under the + * name it is written with, so a folded spelling would open a second, + * phantom object beside the one it meant. * 5. The target lies inside the written package's dependency closure — the * package's own objects, its declared dependencies' and platform objects * (`narrowObjectsToPackageClosure`, the one predicate), refused with the @@ -195,6 +199,14 @@ export function objectExtensionItemRefusal( 'extend', ); } + if (typeof object.name === 'string' && object.name !== target) { + return refusal( + `Object extension '${name}' extends '${target}', but that object is registered as '${object.name}': ` + + `an extension is filed under the name it is written with, so '${target}' would open a second object ` + + `beside '${object.name}'. Save it as '${object.name}', with \`extend\` set to '${object.name}'.`, + 'extend', + ); + } if (narrowObjectsToPackageClosure([object], scope).length === 0) { return refusal( `Object extension '${name}' in package '${binding}': object '${target}' is outside the package's ` diff --git a/packages/metadata-protocol/src/protocol-packaged-base-refusal.ts b/packages/metadata-protocol/src/protocol-packaged-base-refusal.ts index 3027ff4155c..7cb9b02e329 100644 --- a/packages/metadata-protocol/src/protocol-packaged-base-refusal.ts +++ b/packages/metadata-protocol/src/protocol-packaged-base-refusal.ts @@ -38,6 +38,7 @@ import type { ObjectStackProtocolImplementation } from './protocol.js'; import { isOriginGatedType, managedItemSealedSentence, regimeCDeclaredSaveSentence } from './packaged-base-regime.js'; import { SysMetadataRepository } from './sys-metadata-repository.js'; +import { OBJECT_EXTENSION_TYPE } from './protocol-object-extension-item.js'; import { customizationBasePackageId, customizationPackageId, @@ -330,6 +331,9 @@ export interface OverrideWriteClosure { readonly shippers: readonly string[]; } +/** [#28834] The item door an `object_extension` is saved through (#28574), as the refusals name it. */ +const OBJECT_EXTENSION_ITEM_DOOR = '`/api/v1/meta/object_extension`'; + /** Which named refusal of ADR-0146 D2 / D6 / D9 / D10 a write meets. */ export type OverrideWriteRule = | 'undeclared_same_name' @@ -411,9 +415,20 @@ export function judgeOverrideWrite( if (declaration.present) { const row = getOverrideTypeClassification(type); if (row?.disposition === 'extend_only') { + // [#28834] Both doors an extension is written through: the item + // door (#28574) and the manifest's collection. return refuse('declared_object', `${item} carries \`overrides\`, but an object is extended, never replaced (ADR-0146 D10). ` - + 'Remove `overrides` and add the change as an `objectExtensions` entry of the base\'s paired .custom package.'); + + 'Remove `overrides` and add the change as an object extension in the base\'s paired .custom package: ' + + `an \`object_extension\` item saved through ${OBJECT_EXTENSION_ITEM_DOOR}, or an \`objectExtensions\` entry of its manifest.`); + } + if (type === OBJECT_EXTENSION_TYPE) { + // [#28834, ADR-0146 D10] An extension takes no override and gets + // no new name: its name is the object it extends, so the D6 + // "new similar" remedy below would name a write that cannot exist. + return refuse('declared_type_not_overridable', + `${item} carries \`overrides\`, but an object extension takes no override (ADR-0146 D10): it adds to the ` + + `object it extends and is stored under that object's name. Remove \`overrides\` and edit the object_extension '${name}' itself.`); } if (row?.disposition !== 'overridable') { return refuse('declared_type_not_overridable', 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 new file mode 100644 index 00000000000..cf3f0e3b560 --- /dev/null +++ b/packages/runtime/src/domains/meta-object-extension-item-door-fit.test.ts @@ -0,0 +1,201 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#28834, ADR-0146 D10] An `object_extension` item's refusals fit the item + * door #28574 opened. + * + * The business case: an administrator adds "Region" to the vendor's Account + * through Setup, which saves an `object_extension` item into the base's + * `.custom` pair. + * + * 1. Carrying `overrides`, the save is refused (`declared_type_not_overridable`) + * with a sentence that fits an extension: it takes no override and has no + * new name, so the remedy is to edit the extension itself. An `object` + * carrying `overrides` (`declared_object`) is told both doors an + * extension is written through: the item door and the manifest entry. + * Every other type's sentence is unchanged (the permission control). + * 2. A short `extend` that `getObject` folds to a legacy `__` key + * is refused: the registry files an extension under the name it is + * written with, so it would open a second, phantom object. The key the + * object is registered under is still admitted (the control). + * + * ## The rig + * + * The rig of `meta-object-extension-item.test.ts`: better-sqlite3 `:memory:` + * behind the real `ObjectQL` engine and a real + * `ObjectStackProtocolImplementation` over its `SchemaRegistry`. The base is a + * code package registered through `registerApp`; the pair is installed + * through the protocol's package door. Every refusal asserts its envelope, its + * sentence and that no row was left. + */ + +import { describe, it, expect, beforeAll, afterAll, afterEach } from 'vitest'; +import { mkdtempSync, rmSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { ObjectQL } from '@objectstack/objectql'; +import { SqlDriver } from '@objectstack/driver-sql'; +import { ObjectStackProtocolImplementation } from '@objectstack/metadata-protocol'; +import { SysMetadata, SysMetadataHistoryObject, SysMetadataAuditObject } from '@objectstack/platform-objects/metadata'; + +let home: string; +let priorHome: string | undefined; +beforeAll(() => { + priorHome = process.env.OS_HOME; + home = mkdtempSync(join(tmpdir(), 'os-object-extension-door-fit-')); + process.env.OS_HOME = home; +}); +afterAll(() => { + if (priorHome === undefined) delete process.env.OS_HOME; + else process.env.OS_HOME = priorHome; + rmSync(home, { recursive: true, force: true }); +}); + +const TEST_PACKAGE_ID = 'objectstack-test'; +const BASE = 'com.acme.crm'; +const PAIR = 'com.acme.crm.custom'; +const TARGET = 'crm_account'; +/** An object the base ships under a legacy `__` key: `getObject('case')` folds to it. */ +const LEGACY_KEY = 'crm__case'; +const LEGACY_SHORT = 'case'; +const DECLARATION = { package: BASE, basedOn: '3.4.1' }; + +const baseManifest = () => ({ + id: BASE, + name: 'crm', + version: '3.4.1', + type: 'app', + namespace: 'crm', + objects: [ + { name: TARGET, label: 'Account', fields: { name: { name: 'name', label: 'Name', type: 'text' } } }, + { name: LEGACY_KEY, label: 'Case', fields: { subject: { name: 'subject', label: 'Subject', type: 'text' } } }, + ], +}); + +const region = (extend: string, extra: Record = {}) => ({ + extend, + fields: { region__c: { type: 'text', label: 'Region' } }, + ...extra, +}); + +const liveEngines: ObjectQL[] = []; +afterEach(async () => { + while (liveEngines.length) { + try { await liveEngines.pop()?.destroy(); } catch { /* noop */ } + } +}); + +async function boot() { + const engine = new ObjectQL(); + liveEngines.push(engine); + engine.registerDriver(new SqlDriver({ + client: 'better-sqlite3', + connection: { filename: ':memory:' }, + useNullAsDefault: true, + }), true); + await engine.init(); + engine.registry.registerObject(SysMetadata as any, TEST_PACKAGE_ID); + engine.registry.registerObject(SysMetadataHistoryObject as any, TEST_PACKAGE_ID); + engine.registry.registerObject(SysMetadataAuditObject as any, TEST_PACKAGE_ID); + (engine.registry as unknown as { logLevel: string }).logLevel = 'silent'; + engine.registerApp(baseManifest()); + await engine.syncSchemas(); + + const protocol = new ObjectStackProtocolImplementation(engine as any, () => new Map()); + const p = protocol as any; + await p.installPackage({ + manifest: { id: PAIR, name: 'crm-custom', version: '0.1.0', type: 'app', namespace: 'crm_custom', dependencies: { [BASE]: '^3' } }, + }); + + /** Every stored row, read back from the store. */ + const rows = async () => ((await engine.find('sys_metadata', { where: {}, context: { isSystem: true } })) ?? []) + .filter((r: any) => r.package_id === PAIR) + .map((r: any) => ({ type: r.type, name: r.name, state: r.state })); + const thrown = (run: Promise) => run.then(() => undefined, (e: any) => e); + return { engine, p, rows, thrown }; +} + +const envelope = (e: any) => (e ? { code: e.code, status: e.status } : null); +const REFUSED = { code: 'NOT_OVERRIDABLE', status: 403 }; + +describe('an object_extension item\'s override refusals fit the item door (#28834)', () => { + it('`overrides` on an object_extension is refused with the edit-the-extension sentence', async () => { + const { p, rows, thrown } = await boot(); + const e = await thrown(p.saveMetaItem({ + type: 'object_extension', name: TARGET, packageId: PAIR, item: region(TARGET, { overrides: DECLARATION }), + })); + expect(envelope(e)).toEqual(REFUSED); + expect(e.message).toBe( + `'object_extension/${TARGET}' carries \`overrides\`, but an object extension takes no override (ADR-0146 D10): ` + + 'it adds to the object it extends and is stored under that object\'s name. ' + + `Remove \`overrides\` and edit the object_extension '${TARGET}' itself.`, + ); + expect(e.message).not.toMatch(/new name|new similar/); + expect(await rows()).toEqual([]); + }); + + it('`overrides` on an object is refused naming the item door beside the manifest entry', async () => { + const { p, rows, thrown } = await boot(); + const e = await thrown(p.saveMetaItem({ + type: 'object', name: TARGET, packageId: PAIR, + item: { name: TARGET, label: 'Account', fields: {}, overrides: DECLARATION }, + })); + expect(envelope(e)).toEqual(REFUSED); + expect(e.message).toBe( + `'object/${TARGET}' carries \`overrides\`, but an object is extended, never replaced (ADR-0146 D10). ` + + 'Remove `overrides` and add the change as an object extension in the base\'s paired .custom package: ' + + 'an `object_extension` item saved through `/api/v1/meta/object_extension`, or an `objectExtensions` entry of its manifest.', + ); + expect(await rows()).toEqual([]); + }); + + it('control: another excluded type keeps its D6 sentence byte for byte', async () => { + const { p, rows, thrown } = await boot(); + const e = await thrown(p.saveMetaItem({ + type: 'permission', name: 'crm_perm', packageId: PAIR, + item: { name: 'crm_perm', label: 'CRM', overrides: DECLARATION }, + })); + expect(envelope(e)).toEqual(REFUSED); + expect(e.message).toBe( + '\'permission/crm_perm\' carries `overrides`, but a permission takes no override (ADR-0146 D6). ' + + 'Remove `overrides` and create your own permission under a new name ("new similar").', + ); + expect(await rows()).toEqual([]); + }); +}); + +describe('an object_extension item names its target by the key it is registered under (#28834)', () => { + it('a short name getObject folds to a legacy key is refused, naming both spellings, and opens no phantom object', async () => { + const { engine, p, rows, thrown } = await boot(); + // The fold this refuses: the short name reaches the legacy entry. + expect((engine.registry.getObject(LEGACY_SHORT) as any)?.name).toBe(LEGACY_KEY); + + for (const mode of ['draft', undefined] as const) { + const e = await thrown(p.saveMetaItem({ + type: 'object_extension', name: LEGACY_SHORT, packageId: PAIR, item: region(LEGACY_SHORT), + ...(mode ? { mode } : {}), + })); + expect(envelope(e)).toEqual({ code: 'VALIDATION_ERROR', status: 400 }); + expect(e.message).toBe( + `Object extension '${LEGACY_SHORT}' extends '${LEGACY_SHORT}', but that object is registered as '${LEGACY_KEY}': ` + + `an extension is filed under the name it is written with, so '${LEGACY_SHORT}' would open a second object ` + + `beside '${LEGACY_KEY}'. Save it as '${LEGACY_KEY}', with \`extend\` set to '${LEGACY_KEY}'.`, + ); + expect(e.issues?.map((i: any) => i.path)).toEqual(['extend']); + } + expect(await rows()).toEqual([]); + expect(engine.registry.getObjectContributors(LEGACY_SHORT)).toEqual([]); + expect(engine.registry.getObjectContributors(LEGACY_KEY).map((c: any) => c.packageId)).toEqual([BASE]); + }); + + it('control: the key the object is registered under is admitted and extends that object', async () => { + const { engine, p, rows } = await boot(); + 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 }); + expect(await rows()).toEqual([{ type: 'object_extension', name: LEGACY_KEY, state: 'active' }]); + 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'])); + expect(engine.registry.getObjectContributors(LEGACY_SHORT)).toEqual([]); + }); +});