Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions .changeset/28159-residual-picklist-unreferenced.md
Original file line number Diff line number Diff line change
@@ -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

<!-- adr-0087: not-required (no-migration-prescription) No key of any schema is added, removed, renamed or re-shaped, and the picklist schema accepts what it accepted. The boot refuses one composition it used to accept silently, and the refusal names its own remedy; there is no metadata for a migration to rewrite. -->

**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.
139 changes: 139 additions & 0 deletions packages/objectql/src/manifest-service-residual-objects.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<unknown>): Promise<unknown> => 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<ManifestService>('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<ManifestService>('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']);
});
});
65 changes: 64 additions & 1 deletion packages/objectql/src/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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`).
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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<string>();
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 ?? '<none>';
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.
*
Expand Down Expand Up @@ -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
Expand Down
Loading