Skip to content

Commit 2821e9f

Browse files
feat(cli)!: os validate and os build refuse a picklistExtensions entry whose extend names no declared picklist (#21049)
Fixes #20825 Clause-②: no (narrowing) This PR is the item-2 sibling that triage folded into the card (answer `5923151807`): a `picklistExtensions[].extend` that names no picklist is refused by the same judge on the same two doors as a field's dangling `picklist`. It is one more walk, over a second collection, in the file PR #21003 landed (`b84b240b2a`). Nothing else is on the card: the generator and scaffold half went to #21018. ## What changes **`os validate` and `os build` refuse an extension whose `extend` names no picklist the stack declares.** `PicklistExtensionSchema` checks the spelling of `extend` and nothing else, so `extend: 'industy'` beside `picklists: [{ name: 'industry', ... }]` parsed. `os validate` exited 0 and `os build` wrote the artifact carrying the extension. Now both exit 1 with `picklist-reference-unknown`; the finding names the extension (`where`), the list it names (`message`), the lists the stack does declare, and the `path` (`picklistExtensions[N].extend`, prefixed with `packages[I].manifest.` for a package body). - **Same walk as the field reference, one collection added.** A one-package stack is judged on its top-level `picklistExtensions`. A `packages[]` stack is judged on each body's own `picklistExtensions`, and a top-level `picklistExtensions` beside `packages[]` is not judged, because the load path does not register from it (pinned). An `extend` resolves against every picklist the stack declares, including one a sibling package in the same artifact owns. - **Same outside-dependency arm.** When the package declaring the extension lists a `manifest.dependencies` entry the stack does not carry, an unresolved `extend` is an `info` notice (`picklist-reference-unverified`) in `warnings` and on the console, naming the extension, the list and the dependencies. It never gates, not even under `--strict`. That package's own `dependencies` decide, not a sibling's. The coarse trigger itself (any outside id downgrades every unresolved reference in that package) is unchanged: it stays the recorded line triage kept. **Rule id: reuse `picklist-reference-unknown` and `picklist-reference-unverified`, no new sibling id.** What is judged is one fact, a name that resolves to no picklist the stack declares, with one cure class (declare the list or correct the name) and the same two verdicts. The finding's `where`, `path` and message already say which key carried it, and the sentences that differ (`extend: '...'` instead of `picklist: '...'`, "the options it adds have no list to join", a hint that offers removing the entry instead of inline `options`) are chosen per site inside the one judge. A second id would split one verdict in two for everything keyed on the rule: the `--json` `errors[].rule` consumers, the door pins, and the already-pending changeset that names `picklist-reference-unknown`. Nothing in the repo registers these ids in a ledger. **The two doors' refusal heading no longer says "A field".** `validate.ts` (step 2d) and `compile.ts` (step 3a-bis) printed "A field names a picklist this stack does not declare (N references)" above the refusals, which would be false for an `extend` refusal. It now reads "A picklist reference names a picklist this stack does not declare (N references)". That literal and the two step comments are the only edits in those files; the call and the notice printing are unchanged, so `validate-build-gate-parity.test.ts` asked for no registration (no new identifier is called from either command; the whole cli unit tier is green). `packages/spec/**`, `packages/lint/**`, the generator and scaffold, and `collectMetadataStats` are untouched. ## Before / after (real `os` processes on scratch fixtures, run-dev + tsx) Stack: `picklists: [{ name: 'industry', ... }]`, a field `picklist: 'industry'`, and one `picklistExtensions` entry. Before is `origin/main` at `2f2fa11d75`; after is `69f413fb76`. | reading | before | after | |:--|:--|:--| | `os validate`, `extend: 'industy'` | exit 0, "Validation passed" | exit 1, `picklist-reference-unknown` naming `picklist extension "industy"` and `'industy'`, at `picklistExtensions[0].extend` | | `os build`, same stack | exit 0, artifact written carrying `industy` | exit 1, no artifact | | `os validate` / `os build`, `extend: 'industry'` (the control) | exit 0 / 0 | exit 0 / 0, artifact written | After text from `os validate`: ``` ✗ A picklist reference names a picklist this stack does not declare (1 reference) • picklist extension "industy": `extend: 'industy'` names no picklist this stack declares (declared: 'industry'), so the options it adds have no list to join. Correct `extend` to the picklist this entry adds options to, or declare that list — `picklists: [{ name: 'industy', label, options }]` in the package that owns it, or a `*.picklist.ts` file the stack imports. Or remove the entry: it adds options to nothing. rule: picklist-reference-unknown at picklistExtensions[0].extend ``` ## Tests - `packages/cli/src/utils/picklist-references.test.ts` (unit, 19 now, 10 new): the control (a resolving `extend`), the refusal naming extension, list, declared lists and path, a stack that declares no picklist, a field and an extension judged in one pass without hiding each other, the outside-dependency notice, sibling-package resolution, the declaring package's own dependencies, and a top-level `picklistExtensions` beside `packages[]` not judged. - `packages/cli/test/picklist-reference-doors.test.ts` (integration: it spawns the CLI, so the PR tiers run it; 11 now, 5 new): a resolving `extend` validates and builds with the artifact carrying it as authored (the control); a dangling `extend` is refused by `os validate` (JSON and text face) and by `os build`, which writes no artifact. - Ablations on the committed fix, through `scripts/ablation-replace.mjs` under the verify lock. Each mutation landed (anchor 1 to 0, blob changed) and each restore was proven (blob equal to HEAD, `git diff HEAD` empty). The suites read `src` (vitest imports the module; `bin/run-dev.js` runs `src` through tsx), so there is no build leg. - New walk disabled (`if (false && Array.isArray(extensions))`): unit 7 red and 12 green; doors 3 red (JSON, text, `os build`) and 8 green (both control pins and the six field pins). - Package branch disabled (`if (false && parsed.packages !== undefined)`): the new top-level-not-judged pin goes red, along with the two per-package pins that depend on the branch (5 red, 14 green). - At `69f413fb76`: `os-verify-lock` runs of the door file 11/11 and `@objectstack/cli` typecheck exit 0 (including `check:test-typecheck`); the unit pin 19/19; `pnpm lint` exit 0 (full repo, `eslint . --no-inline-config`). The whole `@objectstack/cli` unit tier, 240 files / 3411 tests, was green at `ec1d965497`, one commit earlier; the delta to the head is one comment sentence in `picklist-references.ts`. - Gates: `dispatch-gates --commands` derived 63 families at `69f413fb76`; all 63 were run, each exit code recorded before any pipe, and `--ran` reconciles 63 derived, 63 run, 0 NOT-MEASURED, 0 UNRUN. Four of them (`check:i18n`, `check:i18n-coverage`, `check:i18n-walk-parity`, `check:dual-build-cjs-loads`) first exited 3 (prerequisite not met, nothing measured: no built CLI or workspace dist); after building their prerequisites they were re-run and exited 0 on a final sweep. ## Acceptance notes - A picklist-bound field still raises the `liveness-planned-property` advisory until the runtime reader (#19519) lands; unchanged. - Not judged by this change: the runtime metadata write path (Studio or REST `/meta`). - The metadata summary row for picklists (`collectMetadataStats`) and the coarse outside-dependency trigger stay as triage ruled: the row is #21018's, the trigger a recorded line with no positive case in the repo. --- _Generated by [Claude Code](https://claude.ai/code/session_01VvcEokUG1tvVxkceYfR5XB)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 657b6b7 commit 2821e9f

6 files changed

Lines changed: 411 additions & 58 deletions

File tree

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
---
2+
'@objectstack/cli': minor
3+
---
4+
5+
feat(cli)!: `objectstack validate` and `objectstack build` refuse a `picklistExtensions` entry whose `extend` names no picklist the stack declares (#20825)
6+
7+
Clause-②: no (narrowing — `objectstack validate` / `objectstack build` newly refuse a `picklistExtensions[].extend` that names no picklist the stack declares; nothing is accepted that was refused before)
8+
9+
<!-- adr-0087: not-required (no-migration-prescription) Nothing authorable changes spelling or type: `packages/spec` is untouched, and `picklistExtensions[].extend` stays the snake_case name it was. What changes is that two authoring commands now refuse one authored shape, an extension whose `extend` 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. -->
10+
11+
**BREAKING** — an accept-set narrowing on two authoring commands, shipped as
12+
`minor` under the launch-window convention. A stack with a `picklistExtensions`
13+
entry whose `extend` names no picklist the stack declares — `extend: 'industy'`
14+
beside a `picklists: [{ name: 'industry', … }]` — used to pass `objectstack
15+
validate` and `objectstack build` (which wrote the artifact). Both now exit 1 and
16+
name the extension and the list it names (`picklist-reference-unknown`, the rule a
17+
field's dangling `picklist` already gets).
18+
**One-line fix:** correct `extend` to the picklist the entry adds options to, declare
19+
the list it names (`picklists: [{ name, label, options }]`, or a `*.picklist.ts` file
20+
the stack imports), or remove the entry.
21+
22+
**Which extensions are judged.** The ones the load path registers: the top-level
23+
`picklistExtensions` of a one-package stack, or each `packages[]` entry's own. An
24+
`extend` resolves against every picklist the stack declares, including one a sibling
25+
package in the same artifact owns.
26+
27+
**A list from a package outside the stack is reported, not refused.** When the
28+
package declaring the extension lists a `manifest.dependencies` entry the stack does
29+
not carry, the list may live there, and these commands cannot read it. That
30+
extension is an `info` notice (`picklist-reference-unverified`) in `warnings` and on
31+
the console, naming the extension, the list and the dependencies — never a failure,
32+
not even under `--strict`.

‎packages/cli/src/commands/compile.ts‎

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -459,11 +459,12 @@ export default class Compile extends Command {
459459
this.exit(1);
460460
}
461461

462-
// 3a-bis. A field `picklist` that names no picklist the stack declares is
463-
// REFUSED — the SAME call `os validate` makes at its step 2d, so the
464-
// two doors cannot disagree about which references resolve. Without
465-
// it this door wrote the artifact carrying the misspelt reference, and
466-
// the command that ships shipped a choice with nothing to choose.
462+
// 3a-bis. A field `picklist`, or a `picklistExtensions` entry's `extend`,
463+
// that names no picklist the stack declares is REFUSED — the SAME call
464+
// `os validate` makes at its step 2d, so the two doors cannot disagree
465+
// about which references resolve. Without it this door wrote the
466+
// artifact carrying the misspelt reference, and the command that ships
467+
// shipped a choice with nothing to choose.
467468
//
468469
// A reference the stack cannot resolve while the declaring package
469470
// depends on packages outside the stack is an `info` notice instead
@@ -484,7 +485,7 @@ export default class Compile extends Command {
484485
}
485486
const n = picklistJudgement.refusals.length;
486487
console.log('');
487-
printError(`A field names a picklist this stack does not declare (${n} reference${n > 1 ? 's' : ''})`);
488+
printError(`A picklist reference names a picklist this stack does not declare (${n} reference${n > 1 ? 's' : ''})`);
488489
printAuthoringRuleErrors(picklistJudgement.refusals, { remedy: JSON_FULL_LIST_REMEDY });
489490
this.exit(1);
490491
}

‎packages/cli/src/commands/validate.ts‎

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -420,15 +420,18 @@ export default class Validate extends Command {
420420
this.exit(1);
421421
}
422422

423-
// 2d. A field `picklist` that names no picklist the stack declares is
424-
// REFUSED, naming the field and the list. `FieldSchema` judges the
425-
// name's spelling only, so a misspelt reference parsed, passed this
426-
// door at exit 0 and reached the runtime as a choice with nothing to
427-
// choose — the silence a NAMED list exists to remove.
423+
// 2d. A field `picklist`, or a `picklistExtensions` entry's `extend`, that
424+
// names no picklist the stack declares is REFUSED, naming the field
425+
// (or the extension) and the list. `FieldSchema` and
426+
// `PicklistExtensionSchema` judge the name's spelling only, so a
427+
// misspelt reference parsed, passed this door at exit 0 and reached
428+
// the runtime as a choice with nothing to choose — the silence a NAMED
429+
// list exists to remove.
428430
//
429431
// The walk is the load path's (see `utils/picklist-references.ts`):
430-
// each `packages[]` body's fields, or the top level's when there is
431-
// no `packages[]`, resolved against every picklist the stack declares.
432+
// each `packages[]` body's fields and extensions, or the top level's
433+
// when there is no `packages[]`, resolved against every picklist the
434+
// stack declares.
432435
// A reference that resolves nowhere is refused only when the
433436
// declaring package depends on no package outside the stack; when it
434437
// does, the list may live there, and this command cannot read it — so
@@ -455,7 +458,7 @@ export default class Validate extends Command {
455458
}
456459
const n = picklistJudgement.refusals.length;
457460
console.log('');
458-
printError(`A field names a picklist this stack does not declare (${n} reference${n > 1 ? 's' : ''})`);
461+
printError(`A picklist reference names a picklist this stack does not declare (${n} reference${n > 1 ? 's' : ''})`);
459462
printAuthoringRuleErrors(picklistJudgement.refusals, { remedy: JSON_FULL_LIST_REMEDY });
460463
this.exit(1);
461464
}

‎packages/cli/src/utils/picklist-references.test.ts‎

Lines changed: 163 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,15 +2,17 @@
22

33
/**
44
* `judgePicklistReferences` — the walk `os validate` (step 2d) and `os build`
5-
* (step 3a-bis) run over a field's `picklist` reference.
5+
* (step 3a-bis) run over a field's `picklist` reference and a
6+
* `picklistExtensions` entry's `extend`.
67
*
78
* Pinned here: which references are judged (the load path's reading — the top
89
* level when there is no `packages[]`, each body's own otherwise), what they
910
* resolve against (every picklist the stack declares), and which of the two
1011
* verdicts an unresolved one gets (refused when the declaring package depends
11-
* on nothing outside the stack; an `info` notice when it does). Asserted by
12-
* rule id, severity, the field named in `where`, the list named in `message`
13-
* and the `path` — never by the sentence around them.
12+
* on nothing outside the stack; an `info` notice when it does) — the same two
13+
* verdicts for a field and for an extension. Asserted by rule id, severity,
14+
* the field or extension named in `where`, the list named in `message` and the
15+
* `path` — never by the sentence around them.
1416
*/
1517

1618
import { describe, expect, it } from 'vitest';
@@ -30,6 +32,12 @@ const industry = {
3032
options: [{ label: 'Technology', value: 'technology' }],
3133
};
3234

35+
/** An extension adding one option to the list it names. */
36+
const extension = (extend: string) => ({
37+
extend,
38+
options: [{ label: 'Healthcare', value: 'healthcare' }],
39+
});
40+
3341
const account = (picklist: string) => ({
3442
name: 'pk_account',
3543
label: 'Account',
@@ -170,3 +178,154 @@ describe('judgePicklistReferences — a `packages[]` stack', () => {
170178
expect(notices.map((n) => n.path)).toEqual(['packages[2].manifest.objects[0].fields.industry.picklist']);
171179
});
172180
});
181+
182+
describe('judgePicklistReferences — a `picklistExtensions[].extend` in a one-package stack', () => {
183+
it('CONTROL: an extension naming a picklist the stack declares is neither refused nor reported', () => {
184+
expect(judgePicklistReferences({
185+
manifest: manifest('com.example.pk'),
186+
picklists: [industry],
187+
picklistExtensions: [extension('industry')],
188+
})).toEqual({ refusals: [], notices: [] });
189+
});
190+
191+
it('refuses an `extend` that names no picklist in the stack, naming the extension and the list', () => {
192+
const { refusals, notices } = judgePicklistReferences({
193+
manifest: manifest('com.example.pk'),
194+
picklists: [industry],
195+
picklistExtensions: [extension('industry'), extension('industy')],
196+
});
197+
expect(notices).toEqual([]);
198+
// Only the second entry: the first resolves.
199+
expect(refusals).toHaveLength(1);
200+
expect(refusals[0]).toMatchObject({
201+
severity: 'error',
202+
rule: PICKLIST_REFERENCE_UNKNOWN,
203+
where: 'picklist extension "industy"',
204+
path: 'picklistExtensions[1].extend',
205+
});
206+
expect(refusals[0].message).toContain("'industy'");
207+
// The lists it could have meant are named, so the typo is visible.
208+
expect(refusals[0].message).toContain("'industry'");
209+
});
210+
211+
it('refuses a dangling `extend` in a stack that declares no picklist at all', () => {
212+
const { refusals } = judgePicklistReferences({
213+
manifest: manifest('com.example.pk'),
214+
picklistExtensions: [extension('industry')],
215+
});
216+
expect(refusals.map((r) => r.rule)).toEqual([PICKLIST_REFERENCE_UNKNOWN]);
217+
expect(refusals[0].path).toBe('picklistExtensions[0].extend');
218+
});
219+
220+
it('judges a field and an extension in one pass: each dangling reference is its own refusal', () => {
221+
const { refusals } = judgePicklistReferences({
222+
manifest: manifest('com.example.pk'),
223+
picklists: [industry],
224+
objects: [account('industy')],
225+
picklistExtensions: [extension('regoin')],
226+
});
227+
expect(refusals.map((r) => r.path)).toEqual([
228+
'objects[0].fields.industry.picklist',
229+
'picklistExtensions[0].extend',
230+
]);
231+
expect(new Set(refusals.map((r) => r.rule))).toEqual(new Set([PICKLIST_REFERENCE_UNKNOWN]));
232+
});
233+
234+
it('a resolving extension does not hide a dangling field reference, nor the reverse', () => {
235+
const fieldOnly = judgePicklistReferences({
236+
manifest: manifest('com.example.pk'),
237+
picklists: [industry],
238+
objects: [account('industy')],
239+
picklistExtensions: [extension('industry')],
240+
});
241+
expect(fieldOnly.refusals.map((r) => r.where)).toEqual(['field "pk_account.industry"']);
242+
243+
const extensionOnly = judgePicklistReferences({
244+
manifest: manifest('com.example.pk'),
245+
picklists: [industry],
246+
objects: [account('industry')],
247+
picklistExtensions: [extension('industy')],
248+
});
249+
expect(extensionOnly.refusals.map((r) => r.where)).toEqual(['picklist extension "industy"']);
250+
});
251+
252+
it('REPORTS, never refuses, when the stack depends on a package it does not carry', () => {
253+
const { refusals, notices } = judgePicklistReferences({
254+
manifest: manifest('com.example.pk', { dependencies: { 'com.acme.crm': '^1.0.0' } }),
255+
picklistExtensions: [extension('industry')],
256+
});
257+
expect(refusals).toEqual([]);
258+
expect(notices).toHaveLength(1);
259+
expect(notices[0]).toMatchObject({
260+
severity: 'info',
261+
rule: PICKLIST_REFERENCE_UNVERIFIED,
262+
path: 'picklistExtensions[0].extend',
263+
});
264+
// The notice is printed without its `where`, so it names the extension, the
265+
// list and the dependency it could not read.
266+
expect(notices[0].message).toContain('picklist extension "industry"');
267+
expect(notices[0].message).toContain("'industry'");
268+
expect(notices[0].message).toContain("'com.acme.crm'");
269+
});
270+
});
271+
272+
describe('judgePicklistReferences — a `picklistExtensions[].extend` in a `packages[]` stack', () => {
273+
it('resolves an extension against a picklist a SIBLING package in the same artifact declares', () => {
274+
expect(judgePicklistReferences({
275+
packages: [
276+
{ manifest: { ...manifest('com.example.core'), picklists: [industry] } },
277+
{
278+
manifest: {
279+
...manifest('com.example.orders', { dependencies: { 'com.example.core': '^1.0.0' } }),
280+
picklistExtensions: [extension('industry')],
281+
},
282+
},
283+
],
284+
})).toEqual({ refusals: [], notices: [] });
285+
});
286+
287+
it('refuses when every declared dependency is inside the artifact — none of them declares the list', () => {
288+
const { refusals, notices } = judgePicklistReferences({
289+
packages: [
290+
{ manifest: { ...manifest('com.example.core'), picklists: [industry] } },
291+
{
292+
manifest: {
293+
...manifest('com.example.orders', { dependencies: { 'com.example.core': '^1.0.0' } }),
294+
picklistExtensions: [extension('region')],
295+
},
296+
},
297+
],
298+
});
299+
expect(notices).toEqual([]);
300+
expect(refusals).toHaveLength(1);
301+
expect(refusals[0]).toMatchObject({
302+
rule: PICKLIST_REFERENCE_UNKNOWN,
303+
where: 'picklist extension "region"',
304+
path: 'packages[1].manifest.picklistExtensions[0].extend',
305+
});
306+
});
307+
308+
it('reads the declaring package\'s OWN dependencies: an outside dependency of a sibling does not soften it', () => {
309+
const { refusals, notices } = judgePicklistReferences({
310+
packages: [
311+
{ manifest: { ...manifest('com.example.core', { dependencies: { 'com.acme.crm': '^1.0.0' } }) } },
312+
{ manifest: { ...manifest('com.example.orders'), picklistExtensions: [extension('region')] } },
313+
{
314+
manifest: {
315+
...manifest('com.example.billing', { dependencies: { 'com.acme.crm': '^1.0.0' } }),
316+
picklistExtensions: [extension('segment')],
317+
},
318+
},
319+
],
320+
});
321+
expect(refusals.map((r) => r.path)).toEqual(['packages[1].manifest.picklistExtensions[0].extend']);
322+
expect(notices.map((n) => n.path)).toEqual(['packages[2].manifest.picklistExtensions[0].extend']);
323+
});
324+
325+
it('does not judge a TOP-LEVEL `picklistExtensions` once `packages[]` carries the bodies: the load path does not register from it', () => {
326+
expect(judgePicklistReferences({
327+
packages: [{ manifest: { ...manifest('com.example.core'), picklists: [industry] } }],
328+
picklistExtensions: [extension('industy')],
329+
})).toEqual({ refusals: [], notices: [] });
330+
});
331+
});

0 commit comments

Comments
 (0)