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
23 changes: 23 additions & 0 deletions .changeset/28248-validate-residual-picklist.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
---
'@objectstack/cli': patch
'@objectstack/metadata': patch
'@objectstack/spec': patch
---

fix(cli)!: `os validate` and `os build` refuse a composed stack's top-level picklist that the boot refuses (#28248)

Clause-②: no (narrowing) — `os validate` and `os build` refuse a composed stack whose top level declares a picklist that no package body declares and no object field the engine registers names, which they accepted before; the boot already refuses every such stack, so nothing that boots is newly refused, and nothing is widened
Metadata-protocol: none

<!-- adr-0087: not-required (no-migration-prescription) No key of any schema is added, removed, renamed or re-shaped, and the stack schema accepts what it accepted. The two authoring doors now give, ahead of the boot, a refusal the boot already gives, with the same remedy; there is no metadata for a migration to rewrite. -->

**BREAKING** for `os validate` / `os build` on a composed stack only (one that carries `packages[]`), and only for a stack the boot already refuses.

A composed stack (one that carries `packages[]`) whose top level declares a picklist that no package body declares, and that no object field the engine registers names, is refused at boot (`INVALID_METADATA`). `os validate` and `os build` used to pass the same stack, so the author learned of it only at boot.

- **Now.** Both doors refuse it as `picklist-residual-unreferenced`, located at the picklist's top-level index. The message and prescription are the boot's own words: 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; or remove it from the top level.
- **What counts as naming it** is the boot's set: a field of a body's objects or `objectExtensions`, 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 name nothing, because no door registers them in a composed stack.
- **Unchanged.** A residual picklist that a top-level object's field names is not refused (the boot lists that object and does not serve it). A body field naming a list only the top level declares is `picklist-reference-unknown`, as before. A body-declared picklist and a stack without `packages[]` are not affected.
- **Prose.** The residual-rule header in `@objectstack/metadata` and the permission-set owner gate's docblock in `@objectstack/spec` now state the picklist exception. No schema changes.

No stack that boots is newly refused: every stack this refuses, the boot already refuses.
5 changes: 2 additions & 3 deletions packages/cli/src/commands/compile.ts
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ import type { PermissionSetNameCollisionDiagnostic } from '@objectstack/plugin-s
import { findViewContainerNameRefusals } from '../utils/view-container-names.js';
// A field `picklist` that names no picklist the stack declares — the walk
// `os validate` step 2d runs, over the same judge.
import { judgePicklistReferences, printPicklistReferenceNotices } from '../utils/picklist-references.js';
import { judgePicklistReferences, picklistRefusalHeading, printPicklistReferenceNotices } from '../utils/picklist-references.js';

export default class Compile extends Command {
static override description = 'Compile ObjectStack configuration to JSON artifact';
Expand Down Expand Up @@ -509,9 +509,8 @@ export default class Compile extends Command {
await emitJson({ success: false, errors: picklistJudgement.refusals, warnings: warningsSoFar(), conversions: conversionNotices }, 0, { compact: true });
this.exit(1);
}
const n = picklistJudgement.refusals.length;
console.log('');
printError(`A picklist reference names a picklist this stack does not declare (${n} reference${n > 1 ? 's' : ''})`);
printError(picklistRefusalHeading(picklistJudgement.refusals));
printAuthoringRuleErrors(picklistJudgement.refusals, { remedy: JSON_FULL_LIST_REMEDY });
this.exit(1);
}
Expand Down
5 changes: 2 additions & 3 deletions packages/cli/src/commands/validate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ import { findViewContainerNameRefusals } from '../utils/view-container-names.js'
// A field `picklist` that names no picklist the stack declares — refused, or
// reported when the declaring package depends on packages outside the stack.
// Walked the way the load path registers, like the refusal above.
import { judgePicklistReferences, printPicklistReferenceNotices } from '../utils/picklist-references.js';
import { judgePicklistReferences, picklistRefusalHeading, printPicklistReferenceNotices } from '../utils/picklist-references.js';

export default class Validate extends Command {
static override description =
Expand Down Expand Up @@ -479,9 +479,8 @@ export default class Validate extends Command {
});
this.exit(1);
}
const n = picklistJudgement.refusals.length;
console.log('');
printError(`A picklist reference names a picklist this stack does not declare (${n} reference${n > 1 ? 's' : ''})`);
printError(picklistRefusalHeading(picklistJudgement.refusals));
printAuthoringRuleErrors(picklistJudgement.refusals, { remedy: JSON_FULL_LIST_REMEDY });
this.exit(1);
}
Expand Down
108 changes: 108 additions & 0 deletions packages/cli/src/utils/picklist-references.boot-parity.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,108 @@
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.

/**
* [ADR-0130 D4] `os validate` / `os build` foretell the boot: a composed
* stack's top-level picklist that no package body declares and no object field
* the engine registers names is refused by the boot (the engine's `manifest`
* service, `INVALID_METADATA`), and `judgePicklistReferences` refuses the same
* stacks as `picklist-residual-unreferenced`, in the same words.
*
* The engine does not export its judgement, so the author-time door restates
* it. This file is what keeps the two from drifting. Each stack is judged by
* the door and booted on a real kernel with the real `ObjectQLPlugin`, the way
* `AppPlugin` hands a config boot's stack to the `manifest` service
* (`{ ...manifest, ...stack }`). Where the door refuses one list, the boot
* throws `${message} ${hint}` byte for byte; where the door raises no such
* refusal, the boot does not throw this one.
*/

import { afterEach, describe, expect, it } from 'vitest';
import { ObjectKernel, type Plugin, type PluginContext } from '@objectstack/core';
import { ObjectQLPlugin } from '@objectstack/objectql';

import { judgePicklistReferences, PICKLIST_RESIDUAL_UNREFERENCED } from './picklist-references.js';

type AnyRec = Record<string, unknown>;

const STACK_ID = 'com.example.parity.release';
const manifest = (id: string) => ({ id, name: id.split('.').pop(), version: '1.0.0', type: 'app' });
const status = { name: 'parity_status', label: 'Status', options: [{ label: 'Open', value: 'open' }] };
const unused = { name: 'parity_unused', label: 'Unused', options: [{ label: 'A', value: 'a' }] };
const statusField = { status: { type: 'select', label: 'Status', picklist: 'parity_status' } };
const caseObj = (fields: AnyRec = { subject: { type: 'text', label: 'Subject' } }) =>
({ name: 'parity_case', label: 'Case', sharingModel: 'private', fields });
const noteObj = (fields: AnyRec = { title: { type: 'text', label: 'Title' } }) =>
({ name: 'parity_note', label: 'Note', sharingModel: 'private', fields });

/** Two bodies, a manifest naming neither, and `top` on the top level. */
const composed = (top: AnyRec, coreExtra: AnyRec = {}): AnyRec => ({
manifest: manifest(STACK_ID),
...top,
packages: [
{ manifest: { ...manifest('com.example.parity.core'), objects: [caseObj()], ...coreExtra } },
{ manifest: { ...manifest('com.example.parity.app'), picklists: [status] } },
],
});

/** Every stack the door's pins name, the refused and the controls alike. */
const STACKS: Record<string, AnyRec> = {
'a list nothing names': composed({ picklists: [unused] }),
'a list only a top-level `objectExtensions` entry names': composed({
picklists: [unused], objectExtensions: [{ extend: 'parity_case', fields: { u: { type: 'select', label: 'U', picklist: 'parity_unused' } } }],
}),
'a list only a diverging top-level copy of a body object names': composed({
picklists: [unused], objects: [caseObj({ u: { type: 'select', label: 'U', picklist: 'parity_unused' } })],
}),
'a list only a body `picklistExtensions` entry extends': composed({ picklists: [unused] }, {
picklistExtensions: [{ extend: 'parity_unused', options: [{ label: 'Closed', value: 'closed' }] }],
}),
'a list only a top-level `plugins[]` entry\'s object names': composed({
picklists: [unused], plugins: [{ name: 'parity_plugin', objects: [noteObj({ u: { type: 'select', label: 'U', picklist: 'parity_unused' } })] }],
}),
'CONTROL: a list a residual object\'s field names': composed({
picklists: [unused], objects: [noteObj({ u: { type: 'select', label: 'U', picklist: 'parity_unused' } })],
}),
'CONTROL: a body-declared list repeated on the top level': composed({ picklists: [status] }),
'CONTROL: a body-declared list a body field names': composed({}, { objects: [caseObj(statusField)] }),
'CONTROL: a stack without `packages[]`': { manifest: manifest(STACK_ID), picklists: [unused], objects: [noteObj()] },
};

const kernels: ObjectKernel[] = [];
afterEach(async () => {
while (kernels.length) {
const kernel = kernels.pop()!;
if (kernel.getState() === 'running') await kernel.shutdown();
}
});

/** Boot the stack through the `manifest` service; the boot's refusal, or `undefined`. */
async function bootRefusal(stack: AnyRec): Promise<(Error & { code?: string }) | undefined> {
const kernel = new ObjectKernel({ logger: { level: 'silent' }, gracefulShutdown: false });
kernels.push(kernel);
await kernel.use(new ObjectQLPlugin());
await kernel.use({
name: 'test.parity.app',
dependencies: ['com.objectstack.engine.objectql'],
init: async (ctx: PluginContext) => {
await ctx.getService<{ register(m: unknown): unknown }>('manifest')
.register({ ...(stack.manifest as AnyRec), ...stack });
},
} as Plugin);
return kernel.bootstrap().then(() => undefined, (e: Error & { code?: string }) => e);
}

describe('picklist-residual-unreferenced — the author-time door and the boot agree', () => {
it.each(Object.keys(STACKS))('%s', async (label) => {
const stack = STACKS[label];
const door = judgePicklistReferences(stack).refusals.filter((r) => r.rule === PICKLIST_RESIDUAL_UNREFERENCED);
const boot = await bootRefusal(stack);
if (label.startsWith('CONTROL')) {
expect(door).toEqual([]);
expect(boot?.message ?? '').not.toContain('is declared on the top level of stack');
return;
}
expect(door).toHaveLength(1);
expect(boot?.code).toBe('INVALID_METADATA');
expect(boot?.message).toBe(`${door[0].message} ${door[0].hint}`);
});
});
115 changes: 115 additions & 0 deletions packages/cli/src/utils/picklist-references.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,10 @@
import { describe, expect, it } from 'vitest';
import {
judgePicklistReferences,
picklistRefusalHeading,
PICKLIST_REFERENCE_UNKNOWN,
PICKLIST_REFERENCE_UNVERIFIED,
PICKLIST_RESIDUAL_UNREFERENCED,
} from './picklist-references.js';

const manifest = (id: string, extra: Record<string, unknown> = {}) => ({
Expand Down Expand Up @@ -329,3 +331,116 @@ describe('judgePicklistReferences — a `picklistExtensions[].extend` in a `pack
})).toEqual({ refusals: [], notices: [] });
});
});

/**
* [ADR-0130 D4] A composed stack's top-level picklist that no package body
* declares and no object field the boot registers names: the boot refuses the
* stack (`INVALID_METADATA`), so this door refuses it too, as
* `picklist-residual-unreferenced`, in the boot's words. The boot's own text is
* held equal in `picklist-references.boot-parity.test.ts`.
*/
describe('judgePicklistReferences — a composed stack\'s residual picklist that nothing names', () => {
const STACK_ID = 'com.example.release';
const status = { name: 'status_list', label: 'Status', options: [{ label: 'Open', value: 'open' }] };
const unused = { name: 'unused_list', label: 'Unused', options: [{ label: 'A', value: 'a' }] };
const statusField = { status: { type: 'select', label: 'Status', picklist: 'status_list' } };
const caseObj = (fields: Record<string, unknown> = { subject: { type: 'text', label: 'Subject' } }) =>
({ name: 'pk_case', label: 'Case', fields });
const noteObj = (fields: Record<string, unknown> = { title: { type: 'text', label: 'Title' } }) =>
({ name: 'pk_note', label: 'Note', fields });
/** Two bodies, a manifest naming neither, and `top` on the top level. */
const composed = (top: Record<string, unknown>, coreExtra: Record<string, unknown> = {}) => ({
manifest: manifest(STACK_ID),
...top,
packages: [
{ manifest: { ...manifest('com.example.core'), objects: [caseObj()], ...coreExtra } },
{ manifest: { ...manifest('com.example.app'), objects: [account('industry')], picklists: [industry] } },
],
});
const residualRefusals = (parsed: Record<string, unknown>) =>
judgePicklistReferences(parsed).refusals.filter((r) => r.rule === PICKLIST_RESIDUAL_UNREFERENCED);

it('refuses one, naming the list, the stack and the boot\'s prescription, located at its top-level index', () => {
const { refusals, notices } = judgePicklistReferences(composed({ picklists: [industry, unused] }));
expect(notices).toEqual([]);
expect(refusals).toHaveLength(1);
expect(refusals[0]).toMatchObject({
severity: 'error',
rule: PICKLIST_RESIDUAL_UNREFERENCED,
where: 'picklist "unused_list"',
path: 'picklists[1]',
});
expect(refusals[0].message).toContain(`Picklist 'unused_list' is declared on the top level of stack '${STACK_ID}'`);
expect(refusals[0].message).toContain('no package body declares it and no object field the engine registers names it');
expect(refusals[0].hint).toContain('Declare it inside the `packages[]` entry of the package whose objects use it');
expect(refusals[0].hint).toContain('name it from a field of the object meant to use it');
expect(refusals[0].hint).toContain('remove it from the top level');
});

it('refuses each such list as its own finding', () => {
expect(residualRefusals(composed({ picklists: [unused, { ...unused, name: 'spare_list' }] })).map((r) => r.where))
.toEqual(['picklist "unused_list"', 'picklist "spare_list"']);
});

it('refuses one that only a top-level `objectExtensions` entry names — no door registers it in a composed stack', () => {
expect(residualRefusals(composed({ picklists: [status], objectExtensions: [{ extend: 'pk_case', fields: statusField }] })))
.toHaveLength(1);
});

it('refuses one that only a top-level copy of a body-declared object names, its fields diverging from the body\'s', () => {
expect(residualRefusals(composed({ picklists: [status], objects: [caseObj(statusField)] }))).toHaveLength(1);
});

it('refuses one that only a top-level `plugins[]` entry\'s object names', () => {
expect(residualRefusals(composed({ picklists: [status], plugins: [{ name: 'p', objects: [noteObj(statusField)] }] })))
.toHaveLength(1);
});

it('refuses one that only a body\'s `picklistExtensions` entry extends', () => {
expect(residualRefusals(composed({ picklists: [status] }, {
picklistExtensions: [{ extend: 'status_list', options: [{ label: 'Closed', value: 'closed' }] }],
}))).toHaveLength(1);
});

it('CONTROL: one a residual object\'s field names is not this refusal (the boot lists that object and does not serve it)', () => {
expect(judgePicklistReferences(composed({ picklists: [status], objects: [noteObj(statusField)] })))
.toEqual({ refusals: [], notices: [] });
});

it('CONTROL: one a body object\'s field names is not this refusal — that field is `picklist-reference-unknown`, as before', () => {
const { refusals } = judgePicklistReferences(composed({ picklists: [status] }, { objects: [caseObj(statusField)] }));
expect(refusals.map((r) => r.rule)).toEqual([PICKLIST_REFERENCE_UNKNOWN]);
});

it('CONTROL: a body-declared picklist, repeated on the top level or not, is not residual', () => {
for (const top of [{ picklists: [industry] }, {}]) {
expect(judgePicklistReferences(composed(top))).toEqual({ refusals: [], notices: [] });
}
});

it('CONTROL: a stack without `packages[]` has no residual — its unnamed picklist is not refused', () => {
expect(judgePicklistReferences({ manifest: manifest('com.example.pk'), picklists: [industry, unused], objects: [account('industry')] }))
.toEqual({ refusals: [], notices: [] });
});
});

describe('picklistRefusalHeading — the text face\'s heading names what each rule refused', () => {
const finding = (rule: string) => ({ severity: 'error' as const, rule, where: 'w', path: 'p', message: 'm', hint: 'h' });

it('CONTROL: reference refusals alone keep the heading they always had', () => {
expect(picklistRefusalHeading([finding(PICKLIST_REFERENCE_UNKNOWN), finding(PICKLIST_REFERENCE_UNKNOWN)]))
.toBe('A picklist reference names a picklist this stack does not declare (2 references)');
});

it('a residual refusal is headed as the boot\'s refusal, not as a reference', () => {
const heading = picklistRefusalHeading([finding(PICKLIST_RESIDUAL_UNREFERENCED)]);
expect(heading).toBe('A top-level picklist of this composed stack is one the boot refuses: no package body declares it '
+ 'and no object field the engine registers names it (1 picklist)');
});

it('both kinds together are both named', () => {
const heading = picklistRefusalHeading([finding(PICKLIST_REFERENCE_UNKNOWN), finding(PICKLIST_RESIDUAL_UNREFERENCED)]);
expect(heading).toContain('(1 reference)');
expect(heading).toContain('(1 picklist)');
});
});
Loading
Loading