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
38 changes: 38 additions & 0 deletions .changeset/20825-cli-picklist-kind.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
---
'@objectstack/cli': minor
'@objectstack/lint': patch
---

feat(cli)!: `objectstack validate` and `objectstack build` refuse a field whose `picklist` names no picklist the stack declares, and lint R8 counts `picklist` as an options source (#20825)

Clause-②: no (narrowing — `objectstack validate` / `objectstack build` newly refuse a field `picklist` that names no picklist the stack declares; the R8 change removes a false-positive warning and widens no accept set of its own)

<!-- adr-0087: not-required (no-migration-prescription) Nothing authorable changes spelling or type: `packages/spec` is untouched, and `Field.picklist` stays the snake_case name it was. What changes is that two authoring commands now refuse one authored shape, a field whose `picklist` names no picklist in the stack. `objectstack migrate meta` could not rewrite that shape even in principle, because which list the author meant is not in the metadata. Nothing here judges a stored row. -->

**BREAKING** — an accept-set narrowing on two authoring commands, shipped as
`minor` under the launch-window convention. A stack with a select field whose
`picklist` names no picklist the stack declares — `picklist: 'industy'` beside a
`picklists: [{ name: 'industry', … }]` — used to pass `objectstack validate` and
`objectstack build` (which wrote the artifact). Both now exit 1 and name the field
and the list it names (`picklist-reference-unknown`).
**One-line fix:** correct `picklist` to a list the stack declares, or declare the
list it names (`picklists: [{ name, label, options }]`, or a `*.picklist.ts` file the
stack imports).

**Which references are judged.** The ones the load path registers: the top-level
`objects` and `objectExtensions` of a one-package stack, or each `packages[]`
entry's own. A reference resolves against every picklist the stack declares,
including one a sibling package in the same artifact owns.

**A list from a package outside the stack is reported, not refused.** When the
package declaring the field lists a `manifest.dependencies` entry the stack does not
carry, the list may live there, and these commands cannot read it. That reference is
an `info` notice (`picklist-reference-unverified`) in `warnings` and on the console,
naming the field, the list and the dependencies — never a failure, not even under
`--strict`.

**Lint R8 (`field/select-missing-options`)** no longer reports a select, multiselect
or radio field that names a `picklist`: the picklist is its options source. The
warning it used to give pointed at `options`, which a field naming a `picklist`
cannot add — the field schema refuses the two together. A select with neither still
warns, and its fix now names both sources as alternatives.
41 changes: 41 additions & 0 deletions packages/cli/src/commands/compile.ts
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,9 @@ import type { PermissionSetNameCollisionDiagnostic } from '@objectstack/plugin-s
// [#20393] The boot registrar's divergent view-container `name` refusal — the
// walk `os validate` step 2c runs, over `@objectstack/objectql`'s one judge.
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';

export default class Compile extends Command {
static override description = 'Compile ObjectStack configuration to JSON artifact';
Expand Down Expand Up @@ -178,6 +181,13 @@ export default class Compile extends Command {
// computes the identical record so the residue pin holds, and it reports
// without ever refusing. Appended LAST, in `os validate`'s order.
let jsxGateNotices: ReturnType<typeof resolveJsxGateManifest>['notices'] = [];
// The `info` records of a field `picklist` reference that resolves nowhere
// in the stack while the declaring package depends on packages outside it
// (step 3a-bis). A member of `warningsSoFar()`, ⛔ not a payload key, for
// the reasons the three lists above record: it reports without refusing,
// and `os validate` computes the identical records. Appended LAST, in
// `os validate`'s order.
let picklistReferenceNotices: ReturnType<typeof judgePicklistReferences>['notices'] = [];
const warningsSoFar = () => [
...ruleAdvisories,
...docWarnings,
Expand All @@ -186,6 +196,7 @@ export default class Compile extends Command {
...navGroupWarnings,
...permissionSetCollisionWarnings,
...jsxGateNotices,
...picklistReferenceNotices,
];
// [#18780] ONE rendering of the author-time advisory block, from the
// COMPLETE list — hoisted here for the same reason the lists above are.
Expand Down Expand Up @@ -448,6 +459,36 @@ export default class Compile extends Command {
this.exit(1);
}

// 3a-bis. A field `picklist` that names no picklist the stack declares is
// REFUSED — the SAME call `os validate` makes at its step 2d, so the
// two doors cannot disagree about which references resolve. Without
// it this door wrote the artifact carrying the misspelt reference, and
// the command that ships shipped a choice with nothing to choose.
//
// A reference the stack cannot resolve while the declaring package
// depends on packages outside the stack is an `info` notice instead
// (see `utils/picklist-references.ts`): the list may live there, and
// this command cannot read it. Printed here, carried in
// `warningsSoFar()`, never gating.
//
// Right after the parse and ahead of every artifact write, like 3a.
// The `--json` face is 3a's envelope (`errors`); the text face is
// `os validate`'s.
const picklistJudgement = judgePicklistReferences(result.data as Record<string, unknown>);
picklistReferenceNotices = [...picklistJudgement.notices];
if (!flags.json) printPicklistReferenceNotices(picklistReferenceNotices);
if (picklistJudgement.refusals.length > 0) {
if (flags.json) {
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 field names a picklist this stack does not declare (${n} reference${n > 1 ? 's' : ''})`);
printAuthoringRuleErrors(picklistJudgement.refusals, { remedy: JSON_FULL_LIST_REMEDY });
this.exit(1);
}

// 3b. The author-time rule registry (#4409) — one table, three commands.
// `os build` was the WEAKEST of the three authoring gates before it:
// it published stacks `os validate` or `os lint` refuse, because the
Expand Down
53 changes: 53 additions & 0 deletions packages/cli/src/commands/validate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,10 @@ import type { PermissionSetNameCollisionDiagnostic } from '@objectstack/plugin-s
// over the parsed stack the way the load path registers it. The verdict is
// `@objectstack/objectql`'s; see the module header.
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';

export default class Validate extends Command {
static override description =
Expand Down Expand Up @@ -166,6 +170,13 @@ export default class Validate extends Command {
// of a project without a manifest is unchanged on both faces. `os compile`
// computes the identical record, so the residue pin keeps holding.
let jsxGateNotices: ReturnType<typeof resolveJsxGateManifest>['notices'] = [];
// The `info` records of a field `picklist` reference that resolves nowhere
// in the stack while the declaring package depends on packages outside it
// (step 2d). Same class as the notice above — reports, never refuses, and
// is NOT in the `warnings` list `--strict` reads: the reference may be
// right, and `--strict` failing on it would refuse a correct stack. `os
// compile` computes the identical records, so the residue pin keeps holding.
let picklistReferenceNotices: ReturnType<typeof judgePicklistReferences>['notices'] = [];
const warningsSoFar = () => [
...ruleAdvisories,
...docWarnings,
Expand All @@ -184,6 +195,8 @@ export default class Validate extends Command {
...permissionSetCollisionWarnings,
// [#20113] APPENDED for the same reason, one member later again.
...jsxGateNotices,
// APPENDED for the same reason, one member later again.
...picklistReferenceNotices,
];
// [commit 79cf692b0] The ADR-0087 D2 conversion notices, hoisted for the SAME reason
// and under the SAME ruling as the five lists above — one field over. The
Expand Down Expand Up @@ -407,6 +420,46 @@ export default class Validate extends Command {
this.exit(1);
}

// 2d. A field `picklist` that names no picklist the stack declares is
// REFUSED, naming the field and the list. `FieldSchema` judges the
// name's spelling only, so a misspelt reference parsed, passed this
// door at exit 0 and reached the runtime as a choice with nothing to
// choose — the silence a NAMED list exists to remove.
//
// The walk is the load path's (see `utils/picklist-references.ts`):
// each `packages[]` body's fields, or the top level's when there is
// no `packages[]`, resolved against every picklist the stack declares.
// A reference that resolves nowhere is refused only when the
// declaring package depends on no package outside the stack; when it
// does, the list may live there, and this command cannot read it — so
// that case is an `info` notice, printed here and carried in
// `warningsSoFar()`, never gating.
//
// Right after the parse, ahead of the rule table, for the reason
// step 2c gives. `os build` runs the same call at its step 3a-bis.
const picklistJudgement = judgePicklistReferences(result.data as Record<string, unknown>);
picklistReferenceNotices = [...picklistJudgement.notices];
if (!flags.json) printPicklistReferenceNotices(picklistReferenceNotices);
if (picklistJudgement.refusals.length > 0) {
if (flags.json) {
await emitJson({
valid: false,
errors: picklistJudgement.refusals,
// Every exit carries the lists the run has computed so far — the
// pre-parse ones and the notices above.
warnings: warningsSoFar(),
conversions: conversionNotices,
duration: timer.elapsed(),
});
this.exit(1);
}
const n = picklistJudgement.refusals.length;
console.log('');
printError(`A field names a picklist this stack does not declare (${n} reference${n > 1 ? 's' : ''})`);
printAuthoringRuleErrors(picklistJudgement.refusals, { remedy: JSON_FULL_LIST_REMEDY });
this.exit(1);
}

// 3. The author-time rule registry (#4409). Every rule the three authoring
// commands share — expressions, view shape, widget/action/filter/name
// references, SDUI styling, page sources, security posture, the CLI's
Expand Down
172 changes: 172 additions & 0 deletions packages/cli/src/utils/picklist-references.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,172 @@
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.

/**
* `judgePicklistReferences` — the walk `os validate` (step 2d) and `os build`
* (step 3a-bis) run over a field's `picklist` reference.
*
* Pinned here: which references are judged (the load path's reading — the top
* level when there is no `packages[]`, each body's own otherwise), what they
* resolve against (every picklist the stack declares), and which of the two
* verdicts an unresolved one gets (refused when the declaring package depends
* on nothing outside the stack; an `info` notice when it does). Asserted by
* rule id, severity, the field named in `where`, the list named in `message`
* and the `path` — never by the sentence around them.
*/

import { describe, expect, it } from 'vitest';
import {
judgePicklistReferences,
PICKLIST_REFERENCE_UNKNOWN,
PICKLIST_REFERENCE_UNVERIFIED,
} from './picklist-references.js';

const manifest = (id: string, extra: Record<string, unknown> = {}) => ({
id, name: id, version: '1.0.0', type: 'app', ...extra,
});

const industry = {
name: 'industry',
label: 'Industry',
options: [{ label: 'Technology', value: 'technology' }],
};

const account = (picklist: string) => ({
name: 'pk_account',
label: 'Account',
fields: {
name: { type: 'text', label: 'Name' },
industry: { type: 'select', label: 'Industry', picklist },
},
});

describe('judgePicklistReferences — a one-package stack', () => {
it('CONTROL: a field naming a picklist the stack declares is neither refused nor reported', () => {
expect(judgePicklistReferences({
manifest: manifest('com.example.pk'),
picklists: [industry],
objects: [account('industry')],
})).toEqual({ refusals: [], notices: [] });
});

it('CONTROL: a select with inline options and no `picklist` is not this judge\'s business', () => {
expect(judgePicklistReferences({
manifest: manifest('com.example.pk'),
objects: [{
name: 'pk_account',
fields: { tier: { type: 'select', options: [{ label: 'Gold', value: 'gold' }] } },
}],
})).toEqual({ refusals: [], notices: [] });
});

it('refuses a `picklist` that names no picklist in the stack, naming the field and the list', () => {
const { refusals, notices } = judgePicklistReferences({
manifest: manifest('com.example.pk'),
picklists: [industry],
objects: [account('industy')],
});
expect(notices).toEqual([]);
expect(refusals).toHaveLength(1);
expect(refusals[0]).toMatchObject({
severity: 'error',
rule: PICKLIST_REFERENCE_UNKNOWN,
where: 'field "pk_account.industry"',
path: 'objects[0].fields.industry.picklist',
});
expect(refusals[0].message).toContain("'industy'");
// The lists it could have meant are named, so the typo is visible.
expect(refusals[0].message).toContain("'industry'");
});

it('refuses a dangling reference in a stack that declares no picklist at all', () => {
const { refusals } = judgePicklistReferences({
manifest: manifest('com.example.pk'),
objects: [account('industry')],
});
expect(refusals.map((r) => r.rule)).toEqual([PICKLIST_REFERENCE_UNKNOWN]);
});

it('judges the fields an `objectExtensions` entry merges into its target too', () => {
const { refusals } = judgePicklistReferences({
manifest: manifest('com.example.pk'),
picklists: [industry],
objectExtensions: [{ extend: 'crm_account', fields: { segment: { type: 'select', picklist: 'segment' } } }],
});
expect(refusals).toHaveLength(1);
expect(refusals[0]).toMatchObject({
where: 'field "crm_account.segment"',
path: 'objectExtensions[0].fields.segment.picklist',
});
});

it('REPORTS, never refuses, when the stack depends on a package it does not carry', () => {
const { refusals, notices } = judgePicklistReferences({
manifest: manifest('com.example.pk', { dependencies: { 'com.acme.crm': '^1.0.0' } }),
objects: [account('industry')],
});
expect(refusals).toEqual([]);
expect(notices).toHaveLength(1);
expect(notices[0]).toMatchObject({
severity: 'info',
rule: PICKLIST_REFERENCE_UNVERIFIED,
path: 'objects[0].fields.industry.picklist',
});
// The notice is printed without its `where`, so it names the field, the
// list and the dependency it could not read.
expect(notices[0].message).toContain('pk_account.industry');
expect(notices[0].message).toContain("'industry'");
expect(notices[0].message).toContain("'com.acme.crm'");
});
});

describe('judgePicklistReferences — a `packages[]` stack', () => {
it('resolves a picklist a SIBLING package in the same artifact declares', () => {
expect(judgePicklistReferences({
packages: [
{ manifest: { ...manifest('com.example.core'), picklists: [industry] } },
{
manifest: {
...manifest('com.example.orders', { dependencies: { 'com.example.core': '^1.0.0' } }),
objects: [account('industry')],
},
},
],
})).toEqual({ refusals: [], notices: [] });
});

it('refuses when every declared dependency is inside the artifact — none of them declares the list', () => {
const { refusals, notices } = judgePicklistReferences({
packages: [
{ manifest: { ...manifest('com.example.core'), picklists: [industry] } },
{
manifest: {
...manifest('com.example.orders', { dependencies: { 'com.example.core': '^1.0.0' } }),
objects: [account('region')],
},
},
],
});
expect(notices).toEqual([]);
expect(refusals).toHaveLength(1);
expect(refusals[0]).toMatchObject({
rule: PICKLIST_REFERENCE_UNKNOWN,
path: 'packages[1].manifest.objects[0].fields.industry.picklist',
});
});

it('reads the declaring package\'s OWN dependencies: an outside dependency of a sibling does not soften it', () => {
const { refusals, notices } = judgePicklistReferences({
packages: [
{ manifest: { ...manifest('com.example.core', { dependencies: { 'com.acme.crm': '^1.0.0' } }) } },
{ manifest: { ...manifest('com.example.orders'), objects: [account('region')] } },
{
manifest: {
...manifest('com.example.billing', { dependencies: { 'com.acme.crm': '^1.0.0' } }),
objects: [account('segment')],
},
},
],
});
expect(refusals.map((r) => r.path)).toEqual(['packages[1].manifest.objects[0].fields.industry.picklist']);
expect(notices.map((n) => n.path)).toEqual(['packages[2].manifest.objects[0].fields.industry.picklist']);
});
});
Loading
Loading