Skip to content

Commit bd29ec3

Browse files
committed
fix(lint): refuse a present non-array stack.packages at all five recordsOf(stack.packages) readers
Ruling A on #15293 (comment 5634034754): a `packages` that is present but not an array (`{}`, `0`, `'x'`, a keyed object) is malformed, not absent, and every reader refuses it. Four `packages/lint` readers fell through `recordsOf(stack.packages)` to `[]` instead of refusing: `validate-object-references.ts`'s `artifactProvidedObjectNames`, and `validate-translation-references.ts`'s `contributedNavItemsByApp`, `objectExtensionsByTarget` and `artifactProvidedRecords`. A fifth copy of the same pattern (`validate-mapping-target-fields.ts`'s `extensionFieldsByTarget`) landed via #20208 after this ruling's site census and is fixed the same way. All five now share one small reader, `packagesOf(stack)` (`object-graph.ts`): absent stays `[]`, an array is read exactly as `recordsOf` read it (junk entries dropped, unchanged), and anything else present throws `INVALID_ARTIFACT_PACKAGES` (ADR-0112, status 422) -- the same registered code `@objectstack/core`'s `resolveArtifactPackageOrder` already raises for the identical defect. `recordsOf` itself is untouched: it stays the shared map-or-array reader `objects`/`sections`/`tabs` need, where a keyed map is legitimate -- `packages` never has a map form, so this is a second, narrower reader rather than a branch on the first one. Pins: `packagesOf` is pinned exhaustively (array control, absent/null silence, refusal on `{}`/`0`/`'x'`/a keyed object, with code + status). Each of the three public functions these readers sit behind (`validateObjectReferences`, `validateTranslationReferences`, `validateMappingTargetFields`) gets one throw-pin proving the wiring reaches the shared reader. One existing pin asserted the old fall-through semantics (`validate-object-references.test.ts`, "ignores a `packages` value that is not a list of entries") and is flipped: `null` stays a silence control, `42`/`'core'` now assert the refusal. Co-authored-by: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QcAS3qiYYZNezaxZxaUdMV
1 parent 4df101c commit bd29ec3

9 files changed

Lines changed: 173 additions & 12 deletions
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
---
2+
"@objectstack/lint": minor
3+
---
4+
5+
`packages/lint`'s four `stack.packages` readers (plus a fifth added by #20208 after this ruling's own site census) now refuse a PRESENT non-array `packages` — `{}`, `0`, `'x'`, or a keyed object — the same way `@objectstack/core`'s `resolveArtifactPackageOrder` already does, instead of silently reading it as "no packages" (#20206, ruling A on #15293, comment 5634034754).
6+
7+
Clause-②: no (narrowing)
8+
9+
<!-- adr-0087: not-required (no-migration-prescription) an already-malformed `packages` value is refused rather than converted; no key, export or stored value moves, and nothing in this repo emits the shape today -->
10+
11+
- **What changes**: `validateObjectReferences`, `validateTranslationReferences` and `validateMappingTargetFields` (the three public `@objectstack/lint` functions these readers sit behind) now throw an `INVALID_ARTIFACT_PACKAGES` error (ADR-0112, `status: 422`) instead of returning findings, when the stack they are handed carries a `packages` key that is present but not an array. `os validate` / `os lint` / `os build` surface it as a refusal on stderr (and in `error`/`code` under `--json`) instead of reporting the stack as clean.
12+
- **What does not change**: an absent `packages`, and `packages: null`, are still read as "no packages" — unchanged, and #19926's surface, not this one. A well-formed `packages[]` array is read exactly as before, junk entries dropped exactly as before.
13+
- **Fix**: write `packages` as an array of `{ manifest: … }` entries, or omit the key entirely for a single-package stack.

‎packages/lint/src/object-graph.test.ts‎

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import {
1414
isUnjudgeable,
1515
nearestName,
1616
listNames,
17+
packagesOf,
1718
RELATIONSHIP_FIELD_TYPES,
1819
} from './object-graph.js';
1920
import { walkFilterFieldKeys, type FilterFieldKey } from './filter-walk.js';
@@ -292,6 +293,46 @@ describe('object-graph — a non-record entry in `stack.objects` (#15494)', () =
292293
});
293294
});
294295

296+
describe('object-graph — packagesOf (#20206, ruling A on #15293 `5634034754`)', () => {
297+
// `packages` is declared `z.array(ArtifactPackageSchema).optional()` — array
298+
// or absent, never map-or-array like `objects`/`sections`/`tabs`. A PRESENT
299+
// non-array `packages` ({}, 0, 'x', a keyed object) is malformed, not
300+
// absent, and every reader refuses it. This is the ONE reader the four
301+
// `packages/lint` call sites now share, replacing four private copies of
302+
// `recordsOf(stack.packages)`.
303+
304+
it('CONTROL — an array is read exactly as `recordsOf` read it: iterated, junk dropped', () => {
305+
const valid = { manifest: { id: 'com.example.a' } };
306+
expect(packagesOf({ packages: [valid] })).toEqual([valid]);
307+
expect(packagesOf({ packages: [null, valid, undefined, 'junk', 42, []] })).toEqual([valid]);
308+
});
309+
310+
it('an absent `packages` stays silent — `[]`, not a refusal', () => {
311+
expect(packagesOf({})).toEqual([]);
312+
expect(packagesOf({ packages: undefined })).toEqual([]);
313+
});
314+
315+
it('`packages: null` also stays silent here — #19926 owns that disagreement, not this reader', () => {
316+
expect(packagesOf({ packages: null })).toEqual([]);
317+
});
318+
319+
it.each([
320+
['an empty object', {}],
321+
['a keyed object (the shape `recordsOf` would have read as a map)', { a: { manifest: {} } }],
322+
['a number', 0],
323+
['a string', 'x'],
324+
])('refuses a PRESENT non-array `packages` — %s', (_label, shape) => {
325+
expect(() => packagesOf({ packages: shape })).toThrow(
326+
expect.objectContaining({ code: 'INVALID_ARTIFACT_PACKAGES', status: 422 }),
327+
);
328+
// The message names the actual runtime type, so an author sees what they
329+
// wrote rather than a generic "malformed" sentence.
330+
expect(() => packagesOf({ packages: shape })).toThrow(
331+
new RegExp(`\`packages\` of type ${typeof shape}`),
332+
);
333+
});
334+
});
335+
295336
/**
296337
* [#19289] A `{ type: 'user' }` field with no `reference` is TRAVERSABLE — the
297338
* third defect found by the implicit-target census, and the widest-reaching of

‎packages/lint/src/object-graph.ts‎

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -220,6 +220,59 @@ export function recordsOf(v: unknown): AnyRec[] {
220220
return [];
221221
}
222222

223+
/** The shape a thrown {@link packagesOf} refusal carries (ADR-0112 envelope). */
224+
export type StackPackagesError = Error & { code: string; status: number };
225+
226+
/**
227+
* Every entry of `stack.packages` — the release artifact's package list
228+
* (ADR-0130 D4) — read the way {@link resolveArtifactPackageOrder}
229+
* (`@objectstack/core`) reads it, ⛔ NOT the way {@link recordsOf} reads
230+
* `objects` / `sections` / `tabs`.
231+
*
232+
* `packages` is declared `z.array(ArtifactPackageSchema).optional()`
233+
* (`ObjectStackDefinitionSchema`, `@objectstack/spec`) — array-or-absent,
234+
* never map-or-array. Ruling A on #15293 (`5634034754`): a `packages` that is
235+
* PRESENT but not an array (`{}`, `0`, `'x'`, or a keyed object) is
236+
* **malformed, not absent**, and every reader refuses it — `recordsOf` cannot
237+
* be that reader, because for its OTHER callers a plain object legitimately
238+
* IS the map form (see the note above this function). `packages` has no map
239+
* form at all, so this is a second, narrower reader rather than a branch on
240+
* the first one.
241+
*
242+
* - **Absent** (`undefined` / `null`) → `[]`. A single-package artifact
243+
* contributes nothing here — this answers "what does `packages[]` add",
244+
* never "what does this stack provide". (`null` is left exactly this way
245+
* on purpose — #19926 owns that disagreement, not this function.)
246+
* - **An array** → iterated, non-record members dropped — unchanged from
247+
* what every one of these four call sites did through `recordsOf` before
248+
* this function existed.
249+
* - **Anything else present** → refused, once, here — replacing four copies
250+
* of the same read across `validate-object-references.ts` and
251+
* `validate-translation-references.ts` (#20206).
252+
*
253+
* ⛔ Do not fold this into `recordsOf` itself (#20206's card): that reader
254+
* stays the shared map-or-array reader its other callers need.
255+
*
256+
* @throws A {@link StackPackagesError} — `code: 'INVALID_ARTIFACT_PACKAGES'`,
257+
* `status: 422`, the SAME registered code `resolveArtifactPackageOrder`
258+
* raises for the identical defect on the assembled artifact — never a new
259+
* one.
260+
*/
261+
export function packagesOf(stack: unknown): AnyRec[] {
262+
const declared = (stack as { packages?: unknown } | null | undefined)?.packages;
263+
if (declared === undefined || declared === null) return [];
264+
if (Array.isArray(declared)) return declared.filter(isRec);
265+
const err = new Error(
266+
'A stack\'s `packages` must be an array of package entries (ADR-0130 D4, '
267+
+ '`ArtifactPackageSchema`), but this stack carries `packages` of type '
268+
+ `${typeof declared}. Omit the key entirely for a single-package stack — `
269+
+ '`manifest` is retained, not replaced.',
270+
) as StackPackagesError;
271+
err.code = 'INVALID_ARTIFACT_PACKAGES';
272+
err.status = 422;
273+
throw err;
274+
}
275+
223276
function strName(v: unknown): string | undefined {
224277
return typeof v === 'string' && v.length > 0 ? v : undefined;
225278
}

‎packages/lint/src/validate-mapping-target-fields.test.ts‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,21 @@ describe('validateMappingTargetFields', () => {
103103
expect(validateMappingTargetFields(stack)).toEqual([]);
104104
});
105105

106+
// [#20206, ruling A on #15293 `5634034754`] This reader was added by #20208
107+
// after the ruling's own site census (`origin/main` `1c8b320`) — a fifth
108+
// copy of the same `recordsOf(stack.packages)` fall-through the ruling
109+
// closes elsewhere in this package. A PRESENT non-array `packages` is
110+
// malformed, not absent; only `undefined`/`null` stay silent.
111+
it('refuses a PRESENT non-array `packages` instead of silently ignoring it', () => {
112+
for (const packages of [{}, 0, 'x']) {
113+
expect(() => validateMappingTargetFields({
114+
objects: [contact],
115+
packages,
116+
mappings: [mapping([{ source: 'Tier', target: 'sla_tier' }])],
117+
})).toThrow(expect.objectContaining({ code: 'INVALID_ARTIFACT_PACKAGES', status: 422 }));
118+
}
119+
});
120+
106121
it('stays silent on a mapping whose object this stack does not define (skip 1)', () => {
107122
expect(validateMappingTargetFields({
108123
objects: [contact],

‎packages/lint/src/validate-mapping-target-fields.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@
4848

4949
import { indexImportMappingTargets, unknownImportMappingTargets } from '@objectstack/spec/data';
5050

51-
import { listNames, recordsOf, suggestName } from './object-graph.js';
51+
import { listNames, packagesOf, recordsOf, suggestName } from './object-graph.js';
5252

5353
/** A `fieldMapping[].target` that names no field of the mapping's object. */
5454
export const MAPPING_TARGET_FIELD_UNKNOWN = 'mapping-target-field-unknown';
@@ -92,7 +92,7 @@ function extensionFieldsByTarget(stack: AnyRec): Map<string, AnyRec[]> {
9292
}
9393
};
9494
add(stack.objectExtensions);
95-
for (const entry of recordsOf(stack.packages)) {
95+
for (const entry of packagesOf(stack)) {
9696
if (isRec(entry.manifest)) add(entry.manifest.objectExtensions);
9797
}
9898
return byTarget;

‎packages/lint/src/validate-object-references.test.ts‎

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -297,10 +297,21 @@ describe('validateObjectReferences — artifact packages[] as resolution context
297297
expect(findings[0].path).toBe('objects[0].fields.account.reference');
298298
});
299299

300-
it('ignores a `packages` value that is not a list of entries', () => {
301-
for (const packages of [null, 42, 'core']) {
302-
const findings = validateObjectReferences(perPackageStack(ORDERS_BODY, packages));
303-
expect(findings.map((f) => f.path)).toEqual(['objects[0].fields.account.reference']);
300+
it('`packages: null` stays absent, unlike a present non-array value (#19926 owns `null`, not this rule)', () => {
301+
const findings = validateObjectReferences(perPackageStack(ORDERS_BODY, null));
302+
expect(findings.map((f) => f.path)).toEqual(['objects[0].fields.account.reference']);
303+
});
304+
305+
// [#20206, ruling A on #15293 `5634034754`] Was "ignores a `packages` value
306+
// that is not a list of entries" — `42` and `'core'` used to fall through
307+
// `recordsOf` to `[]` and be silently treated as "no packages", exactly the
308+
// fall-through the ruling closes: PRESENT but not an array is malformed, not
309+
// absent, and every reader refuses it.
310+
it('refuses a PRESENT non-array `packages` instead of silently ignoring it', () => {
311+
for (const packages of [42, 'core']) {
312+
expect(() => validateObjectReferences(perPackageStack(ORDERS_BODY, packages))).toThrow(
313+
expect.objectContaining({ code: 'INVALID_ARTIFACT_PACKAGES', status: 422 }),
314+
);
304315
}
305316
});
306317
});

‎packages/lint/src/validate-object-references.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,7 @@ import {
8181
} from '@objectstack/spec/system';
8282
import { referenceCarrierOf, referenceTargetOf } from '@objectstack/spec/data';
8383

84-
import { recordsOf, suggestName } from './object-graph.js';
84+
import { packagesOf, recordsOf, suggestName } from './object-graph.js';
8585

8686
/** Materialized once for the repeated edit-distance scans in `suggestName`. */
8787
const PLATFORM_NAMES: readonly string[] = [...PLATFORM_PROVIDED_OBJECT_NAMES];
@@ -162,7 +162,7 @@ function isInterpolated(target: string): boolean {
162162
*/
163163
function artifactProvidedObjectNames(stack: AnyRec): string[] {
164164
const names: string[] = [];
165-
for (const entry of recordsOf(stack.packages)) {
165+
for (const entry of packagesOf(stack)) {
166166
const body = entry.manifest;
167167
if (!body || typeof body !== 'object' || Array.isArray(body)) continue;
168168
for (const obj of recordsOf((body as AnyRec).objects)) {

‎packages/lint/src/validate-translation-references.test.ts‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -528,6 +528,34 @@ describe('validateTranslationReferences — cross-package objects (§4 ladder)',
528528
});
529529
});
530530

531+
describe('validateTranslationReferences — a PRESENT non-array `packages` (#20206, ruling A on #15293 `5634034754`)', () => {
532+
// `packages` is declared array-or-absent (ADR-0130 D4), never map-or-array.
533+
// This rule reads it through the three internal carriers `object-graph.ts`'s
534+
// `packagesOf` now serves (contributed nav, object extensions, and every
535+
// artifact-provided collection) — all three now refuse instead of silently
536+
// reading the malformed value as "no packages". `artifactProvidedRecords`
537+
// ('objects') runs first inside `buildUniverse`, so that is the carrier this
538+
// pin observes throwing; the read itself is pinned exhaustively, once, in
539+
// `object-graph.test.ts` (the shared function all three now call).
540+
// A bundle is required — `validateTranslationReferences` returns before ever
541+
// calling `buildUniverse` (and therefore before reading `packages` at all)
542+
// when `stack.translations` is empty, exactly like the exemptions below.
543+
const oneBundle = [{ 'zh-CN': { objects: {} } }];
544+
545+
it('refuses instead of silently treating it as absent', () => {
546+
for (const packages of [{}, 0, 'x', { a: { manifest: {} } }]) {
547+
expect(() => validateTranslationReferences({ objects: [], translations: oneBundle, packages })).toThrow(
548+
expect.objectContaining({ code: 'INVALID_ARTIFACT_PACKAGES', status: 422 }),
549+
);
550+
}
551+
});
552+
553+
it('CONTROL — an absent or `null` `packages` stays silent', () => {
554+
expect(validateTranslationReferences({ objects: [], translations: oneBundle })).toEqual([]);
555+
expect(validateTranslationReferences({ objects: [], translations: oneBundle, packages: null })).toEqual([]);
556+
});
557+
});
558+
531559
describe('validateTranslationReferences — apps, dashboards, global actions', () => {
532560
const stack = {
533561
objects: [{ name: 'crm_lead', fields: { name: { type: 'text' } } }],

‎packages/lint/src/validate-translation-references.ts‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -167,7 +167,7 @@
167167
import { expandViewContainer } from '@objectstack/spec';
168168
import { hasPlatformObjectPrefix, isPlatformProvidedObjectName } from '@objectstack/spec/system';
169169
import { walkFlowNodes } from './flow-walk.js';
170-
import { recordsOf, suggestName } from './object-graph.js';
170+
import { packagesOf, recordsOf, suggestName } from './object-graph.js';
171171
import { walkPageComponents } from './page-walk.js';
172172
import { SYSTEM_FIELDS } from './system-fields.js';
173173
import { viewObjectName } from './view-walk.js';
@@ -750,7 +750,7 @@ function contributedNavItemsByApp(stack: AnyRec): Map<string, unknown[]> {
750750
}
751751
};
752752
add(isRec(stack.manifest) ? stack.manifest.navigationContributions : undefined);
753-
for (const entry of recordsOf(stack.packages)) {
753+
for (const entry of packagesOf(stack)) {
754754
const body = entry.manifest;
755755
if (!isRec(body)) continue;
756756
add(body.navigationContributions);
@@ -845,7 +845,7 @@ function objectExtensionsByTarget(stack: AnyRec): Map<string, AnyRec[]> {
845845
}
846846
};
847847
add(stack.objectExtensions);
848-
for (const entry of recordsOf(stack.packages)) {
848+
for (const entry of packagesOf(stack)) {
849849
const body = entry.manifest;
850850
if (!isRec(body)) continue;
851851
add(body.objectExtensions);
@@ -918,7 +918,7 @@ function objectExtensionsByTarget(stack: AnyRec): Map<string, AnyRec[]> {
918918
*/
919919
function artifactProvidedRecords(stack: AnyRec, collection: string): AnyRec[] {
920920
const provided: AnyRec[] = [];
921-
for (const entry of recordsOf(stack.packages)) {
921+
for (const entry of packagesOf(stack)) {
922922
const body = entry.manifest;
923923
if (!isRec(body)) continue;
924924
provided.push(...recordsOf(body[collection]));

0 commit comments

Comments
 (0)