diff --git a/.changeset/28159-residual-picklist-unreferenced.md b/.changeset/28159-residual-picklist-unreferenced.md new file mode 100644 index 00000000000..d7be4b0698d --- /dev/null +++ b/.changeset/28159-residual-picklist-unreferenced.md @@ -0,0 +1,20 @@ +--- +'@objectstack/objectql': patch +--- + +fix(objectql)!: a composed stack's top-level picklist that no package body declares and no object field names is refused at boot, naming the list and the stack (#28159) + +Clause-②: yes (narrowing) — a stack with `packages[]` whose top level declares a picklist that no package body declares and no object field names booted, and now refuses the boot with `INVALID_METADATA` (422) +Metadata-protocol: none + + + +**BREAKING** for a composed stack only (a stack that carries `packages[]`). A stack without `packages[]` has no residual and is unchanged. + +The engine registers a stack's top-level items that no package body declares (its residual) under the stack's own id, except picklists, which stay metadata only. A residual picklist that an object field names was loud already: a top-level object's field makes that object listed and not served, and a package body's field fails the boot's picklist audit. A residual picklist that no object field names was neither registered nor refused, so the boot booted and the list was missing from the engine with nothing said. + +- **Before.** The boot succeeded and the picklist was absent from the engine registry. +- **Now.** The `manifest` service refuses the artifact before anything of it registers, with `INVALID_METADATA` (422). The message names each such picklist and the stack, and gives the remedy. A field counts as naming the list only when the engine registers it: a field of a package body's object or `objectExtensions` entry, or of a top-level object no body declares. A top-level `objectExtensions` entry, a top-level copy of an object a body declares, and a top-level `plugins[]` entry are not registered in a composed stack, so a list that only they name is refused too, as is one that only a body's `picklistExtensions` entry extends. +- **Unchanged.** A residual picklist that an object field names keeps today's behaviour, a picklist a package body declares registers as before, and a stack with no residual picklist boots as before. + +**The remedy.** Declare the picklist inside the `packages[]` entry of the package whose objects use it; or name it from a field of the object meant to use it; or remove it from the top level. diff --git a/packages/objectql/src/manifest-service-residual-objects.test.ts b/packages/objectql/src/manifest-service-residual-objects.test.ts index 58604687144..0b1522dbce8 100644 --- a/packages/objectql/src/manifest-service-residual-objects.test.ts +++ b/packages/objectql/src/manifest-service-residual-objects.test.ts @@ -542,3 +542,142 @@ describe('ADR-0130 D4 — the manifest service registers every other residual ty } }); }); + +/** + * [ADR-0130 D4] A residual PICKLIST that no object field of the stack names is + * refused at boot. The residual's picklists stay metadata only (the objects + * rule above is judged on that premise), so one that no field names was + * neither registered nor refused: the boot booted, and the list was missing + * from the engine with nothing said. One that a field names is loud already, + * and stays as it is: a residual object's field makes that object listed and + * not served (above), and a body's field fails the boot's picklist audit. + */ +describe('ADR-0130 D4 — a residual picklist that no object field names', () => { + const unusedList = (name = 'acme_unused') => ({ name, label: 'Unused', options: [{ label: 'A', value: 'a' }] }); + const refusalOf = (booting: Promise): Promise => booting.then(() => undefined, (e: unknown) => e); + + it('refuses the boot, naming the picklist and the stack, with the prescription', async () => { + const refusal = await refusalOf(boot(registering('app', [residualStack(noteObj(), { picklists: [unusedList()] })]))); + expect(refusal).toMatchObject({ code: 'INVALID_METADATA', status: 422, httpStatus: 422 }); + const message = (refusal as Error).message; + expect(message).toContain(`Picklist 'acme_unused' is declared on the top level of stack '${RELEASE_ID}'`); + expect(message).toContain('no package body declares it and no object field the engine registers names it'); + expect(message).toContain('Declare it inside the `packages[]` entry of the package whose objects use it'); + expect(message).toContain('name it from a field of the object meant to use it'); + expect(message).toContain('remove it from the top level'); + }); + + it('names every such picklist in one refusal, and not one a field names', async () => { + const refusal = await refusalOf(boot(registering('app', [ + residualStack(noteObj(statusField), { picklists: [statusList(), unusedList(), unusedList('acme_spare')] }), + ]))); + expect(refusal).toMatchObject({ code: 'INVALID_METADATA' }); + const message = (refusal as Error).message; + expect(message).toContain(`Picklists 'acme_unused', 'acme_spare' are declared on the top level of stack '${RELEASE_ID}'`); + expect(message).not.toContain("'acme_status'"); + }); + + it('refuses a stack whose `packages: []` leaves only such a picklist', async () => { + const refusal = await refusalOf(boot(registering('app', [payload({ + manifest: { ...app, id: RELEASE_ID }, + picklists: [unusedList()], + packages: [], + })]))); + expect(refusal).toMatchObject({ code: 'INVALID_METADATA' }); + expect((refusal as Error).message).toContain(`Picklist 'acme_unused' is declared on the top level of stack '${RELEASE_ID}'`); + }); + + it('after start(), refuses the artifact before anything of it registers', async () => { + const { kernel } = await boot(); + const manifest = kernel.getService('manifest'); + const refusal = await refusalOf(Promise.resolve().then(() => + manifest.register(residualStack(noteObj(), { picklists: [unusedList()] })))); + expect(refusal).toMatchObject({ code: 'INVALID_METADATA' }); + expect(owners(engineOf(kernel))).toEqual({ acme_account: undefined, acme_case: undefined, acme_note: undefined }); + expect(acmePackageIds(engineOf(kernel))).toEqual([]); + }); + + /** A residual `acme_status` that only an item the engine never registers in a composed stack names. */ + const isTopLevelOnlyRefusal = (refusal: unknown): void => { + expect(refusal).toMatchObject({ code: 'INVALID_METADATA', status: 422 }); + expect((refusal as Error).message).toContain(`Picklist 'acme_status' is declared on the top level of stack '${RELEASE_ID}'`); + }; + + it('refuses one that only a top-level `objectExtensions` entry names — no door registers that entry in a composed stack', async () => { + isTopLevelOnlyRefusal(await refusalOf(boot(registering('app', [residualStack(noteObj(), { + picklists: [statusList()], + objectExtensions: [{ extend: 'acme_case', fields: statusField }], + })])))); + }); + + it('refuses one that only a top-level copy of a body-declared object names, its fields diverging from the body\'s', async () => { + isTopLevelOnlyRefusal(await refusalOf(boot(registering('app', [payload({ + manifest: { ...app, id: RELEASE_ID }, + objects: [{ ...caseObj(), fields: { ...caseObj().fields, ...statusField } }, accountObj()], + picklists: [statusList()], + packages: [ + { manifest: { ...svc, objects: [caseObj()] } }, + { manifest: { ...app, objects: [accountObj()] } }, + ], + })])))); + }); + + it('refuses one that only a body\'s `picklistExtensions` entry extends — at boot, and after start() before anything registers', async () => { + const extending = () => residualStack(noteObj(), { picklists: [statusList()] }, { + picklistExtensions: [{ extend: 'acme_status', options: [{ label: 'Closed', value: 'closed' }] }], + }); + isTopLevelOnlyRefusal(await refusalOf(boot(registering('app', [extending()])))); + + const { kernel } = await boot(); + isTopLevelOnlyRefusal(await refusalOf(Promise.resolve().then(() => + kernel.getService('manifest').register(extending())))); + expect(owners(engineOf(kernel))).toEqual({ acme_account: undefined, acme_case: undefined, acme_note: undefined }); + expect(acmePackageIds(engineOf(kernel))).toEqual([]); + }); + + it('CONTROL: one a residual object\'s field names keeps the objects rule\'s line — boots, listed and not served, list unregistered', async () => { + const { kernel, recorder } = await boot(registering('app', [ + residualStack(noteObj(statusField), { picklists: [statusList()] }), + ])); + const ql = engineOf(kernel); + expect(kernel.getState()).toBe('running'); + expect(owners(ql).acme_note).toBeUndefined(); + expect(ql.registry.resolvePicklistOptions('acme_status')).toBeUndefined(); + expect(unservedLines(recorder)).toHaveLength(1); + }); + + it('CONTROL: one a BODY object\'s field names still fails the boot\'s picklist audit, not this refusal', async () => { + const refusal = await refusalOf(boot(registering('app', [ + residualStack(noteObj(), { picklists: [statusList()] }, { objects: [{ ...caseObj(), fields: { ...caseObj().fields, ...statusField } }] }), + ]))); + expect(refusal).toMatchObject({ code: 'INVALID_METADATA' }); + const message = (refusal as Error).message; + expect(message).toContain("field 'acme_case.status'"); + expect(message).toContain('that no loaded package declares'); + expect(message).not.toContain('is declared on the top level of stack'); + }); + + it('CONTROL: a picklist a body declares registers as today, whether or not the top level repeats it', async () => { + for (const extra of [{ picklists: [statusList()] }, {}]) { + const { kernel, recorder } = await boot(registering('app', [residualStack(noteObj(), extra, { picklists: [statusList()] })])); + const ql = engineOf(kernel); + expect(kernel.getState()).toBe('running'); + expect(ql.registry.resolvePicklistOptions('acme_status')?.map((o) => o.value)).toEqual(['open']); + expect(unservedLines(recorder)).toEqual([]); + await kernel.shutdown(); + } + }); + + it('CONTROL: a stack with no residual picklist boots unchanged', async () => { + const { kernel, recorder } = await boot(registering('app', [residualStack()])); + expect(kernel.getState()).toBe('running'); + expect(owners(engineOf(kernel))).toEqual({ acme_account: APP_ID, acme_case: SERVICE_ID, acme_note: RELEASE_ID }); + expect(unservedLines(recorder)).toEqual([]); + }); + + it('CONTROL: a stack without `packages[]` has no residual — its unnamed picklist registers as today', async () => { + const { kernel } = await boot(registering('app', [payload({ manifest: app, objects: [noteObj()], picklists: [unusedList()] })])); + expect(kernel.getState()).toBe('running'); + expect(engineOf(kernel).registry.resolvePicklistOptions('acme_unused')?.map((o) => o.value)).toEqual(['a']); + }); +}); diff --git a/packages/objectql/src/plugin.ts b/packages/objectql/src/plugin.ts index 6f18d97b867..1e264be2d2d 100644 --- a/packages/objectql/src/plugin.ts +++ b/packages/objectql/src/plugin.ts @@ -48,6 +48,8 @@ import { collectManifestPicklistNames, collectManifestPicklistReferences, describeUnresolvedPicklistReferences, + picklistMetadataError, + type PicklistMetadataError, } from './picklist-resolution.js'; // [ADR-0048 N.3] The security catalog's one-holder envelope, raised here by the // cold-boot check (see `refuseEnvironmentHeldSecurityCatalogNames`). @@ -170,7 +172,9 @@ function unclaimedTopLevelObjects( * residual declares is what the objects rule lists and does not serve, and * a body's field naming one fails the boot's picklist audit — both loud, * and both judged on that premise, so registering the residual's lists here - * would change which objects the data door serves; + * would change which objects the data door serves. A residual list no + * object field names is refused instead + * ({@link unreferencedResidualPicklistRefusal}); * - `object_extension`: an extension has no slot of its own (it is keyed by * `extend`, and its `name` spelling is a retired alias the schema refuses), * so the residual rule never lists one. @@ -208,6 +212,60 @@ function unclaimedTopLevelItems( : undefined; } +/** + * [ADR-0130 D4] The refusal for a residual PICKLIST that no object field of + * the stack names — `undefined` when there is none. + * + * A residual picklist is kept as metadata only (see + * {@link RESIDUAL_TYPES_WITH_OWN_PATH}), so the engine never registers it. + * One that a field names is loud already: a residual object's field makes + * that object listed and not served, and a body's field fails the boot's + * picklist audit. One that no field names was neither registered nor + * refused, so it went missing from the engine with nothing said. It is + * refused here, before anything of the artifact registers, naming each list + * and the stack. + * + * "Names" is judged off the fields the engine registers from this artifact: + * every package body's (its objects, its `objectExtensions` and its nested + * `plugins[]`) and the residual's own objects, the set + * {@link ObjectQLPlugin.registerUnclaimedTopLevelObjects} judges. ⛔ Never off + * the whole top level: in a composed stack a top-level `objectExtensions` + * entry, a top-level copy of an object a body declares and a top-level + * `plugins[]` entry are registered by no door, so a list only they name is + * dropped all the same. And never off the registry, which is still filling + * while a boot registers. + */ +function unreferencedResidualPicklistRefusal( + residual: UnclaimedTopLevel | undefined, + bodies: readonly unknown[], +): PicklistMetadataError | undefined { + if (!residual) return undefined; + const picklists = residual.items + .filter((entry) => entry.type === 'picklist') + .map((entry) => String((entry.item as { name?: unknown }).name)); + if (picklists.length === 0) return undefined; + const named = new Set(); + const residualObjects = residual.items.filter((entry) => entry.type === 'object').map((entry) => entry.item); + for (const source of [...bodies, { objects: residualObjects }]) { + for (const ref of collectManifestPicklistReferences(source, undefined)) named.add(ref.picklist); + } + const unreferenced = picklists.filter((name) => !named.has(name)); + if (unreferenced.length === 0) return undefined; + const owner = residual.ownerId ?? ''; + const listed = unreferenced.map((name) => `'${name}'`).join(', '); + return picklistMetadataError( + `${unreferenced.length === 1 ? 'Picklist' : 'Picklists'} ${listed} ${unreferenced.length === 1 ? 'is' : 'are'} ` + + `declared on the top level of stack '${owner}', and no package body declares ` + + `${unreferenced.length === 1 ? 'it' : 'them'} and no object field the engine registers names ` + + `${unreferenced.length === 1 ? 'it' : 'them'} (in a composed stack, a top-level \`objectExtensions\` entry, a ` + + 'top-level copy of an object a package body declares and a top-level `plugins[]` entry are not ' + + 'registered). The engine registers a top-level picklist only through a package body, so this list would be missing from the engine with nothing said. Declare it inside the ' + + '`packages[]` entry of the package whose objects use it; or name it from a field of the object meant to ' + + 'use it (a top-level object that does is then listed and not served until both move into a package ' + + 'body); or remove it from the top level.', + ); +} + /** * Options for ObjectQLPlugin. * @@ -831,6 +889,11 @@ export class ObjectQLPlugin implements Plugin { // which readers of the registry alone see only through this. See // {@link unclaimedTopLevelItems}. const residualItems = unclaimedTopLevelItems(residualAnswer); + // …and its picklists, which neither of those registers: one that no + // object field names is refused here, before anything registers. See + // {@link unreferencedResidualPicklistRefusal}. + const unreferencedPicklists = unreferencedResidualPicklistRefusal(residualAnswer, ordered); + if (unreferencedPicklists) throw unreferencedPicklists; if (ordered.length === 0 && residual === undefined && residualItems === undefined) { // An artifact that declared `packages: []` registers nothing. Said out