diff --git a/.changeset/20206-lint-error-code-provenance-row.md b/.changeset/20206-lint-error-code-provenance-row.md new file mode 100644 index 00000000000..4fbaaee3f79 --- /dev/null +++ b/.changeset/20206-lint-error-code-provenance-row.md @@ -0,0 +1,9 @@ +--- +"@objectstack/spec": minor +--- + +`ERROR_CODE_LEDGER['@objectstack/lint']` now lists `INVALID_ARTIFACT_PACKAGES`, the code `packages/lint`'s `packagesOf` reader stamps for a malformed `stack.packages` (#20206) — required by `check:error-code-provenance`, which refuses a registered code stamped by a package whose own owner key does not list it. + +Clause-②: yes + +Provenance, not identity: the code was already registered under `@objectstack/core` (`resolveArtifactPackageOrder`, the producer `packagesOf` deliberately mirrors rather than mints a new code for), so the `ErrorCode` union, the wire, and every other package's rows are unchanged. What widens is the per-package face a consumer reads from `ERROR_CODE_LEDGER['@objectstack/lint']`, newly present where it was absent before. Nothing to migrate. diff --git a/.changeset/20206-lint-packages-non-array-refused.md b/.changeset/20206-lint-packages-non-array-refused.md new file mode 100644 index 00000000000..cd3ff2a56e4 --- /dev/null +++ b/.changeset/20206-lint-packages-non-array-refused.md @@ -0,0 +1,13 @@ +--- +"@objectstack/lint": minor +--- + +`packages/lint`'s five `stack.packages` readers (four named by #20206, plus one added by #20208 after that card's site census) now refuse a PRESENT non-array `packages` — `{}`, `0`, `'x'`, a keyed object, and (as of this round) `null` too — instead of silently reading it as "no packages" (#20206, ruling A on #15293 comment 5634034754; the `null` leg is ruling A on #19926, comment 5805260775: `null` is malformed, everywhere). For every shape other than `null`, this is the same way `@objectstack/core`'s `resolveArtifactPackageOrder` already refuses it, with the same registered code; `@objectstack/core`'s `resolveArtifactPackageOrder` refuses `null` the same way (#19926). + +Clause-②: no (narrowing) + + + +- **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 — `null` included. Only `os lint` reaches this refusal — exit 1, the message on stdout (`printError`), `code` under `--json`; `os validate` and `os build` already refuse a malformed `packages` earlier, at `ObjectStackDefinitionSchema.safeParse`, before these rules ever run. +- **What does not change**: an absent `packages` (the key omitted, or explicitly `undefined`) is still read as "no packages" — unchanged. A well-formed `packages[]` array is read exactly as before, junk entries dropped exactly as before. +- **Fix**: write `packages` as an array of `{ manifest: … }` entries, or omit the key entirely for a single-package stack. diff --git a/packages/lint/src/object-graph.test.ts b/packages/lint/src/object-graph.test.ts index 894bde68bb7..fb8a04a20da 100644 --- a/packages/lint/src/object-graph.test.ts +++ b/packages/lint/src/object-graph.test.ts @@ -14,6 +14,7 @@ import { isUnjudgeable, nearestName, listNames, + packagesOf, RELATIONSHIP_FIELD_TYPES, } from './object-graph.js'; import { walkFilterFieldKeys, type FilterFieldKey } from './filter-walk.js'; @@ -292,6 +293,55 @@ describe('object-graph — a non-record entry in `stack.objects` (#15494)', () = }); }); +describe('object-graph — packagesOf (#20206, ruling A on #15293 `5634034754`)', () => { + // `packages` is declared `z.array(ArtifactPackageSchema).optional()` — array + // or absent, never map-or-array like `objects`/`sections`/`tabs`. A PRESENT + // non-array `packages` ({}, 0, 'x', a keyed object) is malformed, not + // absent, and every reader refuses it. This is the ONE reader the five + // `packages/lint` call sites now share, replacing five private copies of + // `recordsOf(stack.packages)`. + + it('CONTROL — an array is read exactly as `recordsOf` read it: iterated, junk dropped', () => { + const valid = { manifest: { id: 'com.example.a' } }; + expect(packagesOf({ packages: [valid] })).toEqual([valid]); + expect(packagesOf({ packages: [null, valid, undefined, 'junk', 42, []] })).toEqual([valid]); + }); + + it('CONTROL — only an absent (`undefined`) `packages` stays silent — `[]`, not a refusal', () => { + expect(packagesOf({})).toEqual([]); + expect(packagesOf({ packages: undefined })).toEqual([]); + }); + + it.each([ + ['an empty object', {}], + ['a keyed object (the shape `recordsOf` would have read as a map)', { a: { manifest: {} } }], + ['a number', 0], + ['a string', 'x'], + ])('refuses a PRESENT non-array `packages` — %s', (_label, shape) => { + expect(() => packagesOf({ packages: shape })).toThrow( + expect.objectContaining({ code: 'INVALID_ARTIFACT_PACKAGES', status: 422 }), + ); + // The message names the actual runtime type, so an author sees what they + // wrote rather than a generic "malformed" sentence. + expect(() => packagesOf({ packages: shape })).toThrow( + new RegExp(`\`packages\` of type ${typeof shape}`), + ); + }); + + // [ruling A on #19926, `5805260775`] `null` is malformed, everywhere — it is + // PRESENT, not absent, so it takes the same refusal as `{}`/`0`/`'x'`, not + // the silent branch above. `typeof null` is `'object'`, which would name a + // `{}` the author never wrote, so the message names `null` as itself + // (matching `resolveArtifactPackageOrder` in `@objectstack/core`, PR #20228). + it('refuses `packages: null` too — malformed, not absent (ruling `5805260775` on #19926)', () => { + expect(() => packagesOf({ packages: null })).toThrow( + expect.objectContaining({ code: 'INVALID_ARTIFACT_PACKAGES', status: 422 }), + ); + expect(() => packagesOf({ packages: null })).toThrow(/`packages` of type null/); + expect(() => packagesOf({ packages: null })).not.toThrow(/of type object/); + }); +}); + /** * [#19289] A `{ type: 'user' }` field with no `reference` is TRAVERSABLE — the * third defect found by the implicit-target census, and the widest-reaching of diff --git a/packages/lint/src/object-graph.ts b/packages/lint/src/object-graph.ts index bac2db6f0d9..693c48ac130 100644 --- a/packages/lint/src/object-graph.ts +++ b/packages/lint/src/object-graph.ts @@ -220,6 +220,68 @@ export function recordsOf(v: unknown): AnyRec[] { return []; } +/** The shape a thrown {@link packagesOf} refusal carries (ADR-0112 envelope). */ +export type StackPackagesError = Error & { code: string; status: number }; + +/** + * Every entry of `stack.packages` — the release artifact's package list + * (ADR-0130 D4) — read on `packages/lint`'s own terms, ⛔ NOT the way + * {@link recordsOf} reads `objects` / `sections` / `tabs`. + * + * `packages` is declared `z.array(ArtifactPackageSchema).optional()` + * (`ObjectStackDefinitionSchema`, `@objectstack/spec`) — array-or-absent, + * never map-or-array. Ruling A on #15293 (`5634034754`): a `packages` that is + * PRESENT but not an array (`{}`, `0`, `'x'`, or a keyed object) is + * **malformed, not absent**, and every reader refuses it — `recordsOf` cannot + * be that reader, because for its OTHER callers a plain object legitimately + * IS the map form (see the note above this function). `packages` has no map + * form at all, so this is a second, narrower reader rather than a branch on + * the first one. + * + * - **Absent** (`undefined` ONLY) → `[]`. A single-package artifact + * contributes nothing here — this answers "what does `packages[]` add", + * never "what does this stack provide". `null` is malformed, per ruling + * `5805260775` on #19926 — `null` is refused here exactly as + * {@link resolveArtifactPackageOrder} (`@objectstack/core`) refuses it, + * same code and status, since #19926 (PR #20228, `a9fb83ef06`). + * - **An array** → iterated, non-record members dropped — unchanged from + * what every one of these five call sites did through `recordsOf` before + * this function existed. + * - **Anything else present, `null` included** → refused, once, here — + * replacing five copies of the same read across + * `validate-object-references.ts`, `validate-translation-references.ts` + * and `validate-mapping-target-fields.ts` (#20206). + * + * ⛔ Do not fold this into `recordsOf` itself (#20206's card): that reader + * stays the shared map-or-array reader its other callers need. + * + * @throws A {@link StackPackagesError} — `code: 'INVALID_ARTIFACT_PACKAGES'`, + * `status: 422` — for a present non-array `packages` OTHER than `null`, + * the SAME registered code {@link resolveArtifactPackageOrder} already + * raises for the identical defect on the assembled artifact; core raises + * the same code for `null` too (#19926). Never a new code, either way. + */ +export function packagesOf(stack: unknown): AnyRec[] { + const declared = (stack as { packages?: unknown } | null | undefined)?.packages; + // ⛔ `undefined` ONLY. `null` is present, not absent — ruling `5805260775` + // on #19926 — so it falls to the refusal below with every other non-array + // value. + if (declared === undefined) return []; + if (Array.isArray(declared)) return declared.filter(isRec); + const err = new Error( + 'A stack\'s `packages` must be an array of package entries (ADR-0130 D4, ' + + '`ArtifactPackageSchema`), but this stack carries `packages` of type ' + // `typeof null` is `'object'`, which would name a `{}` the author never + // wrote; `null` is named as itself here — the same naming + // `resolveArtifactPackageOrder` uses (`null` named as `null`). + + `${declared === null ? 'null' : typeof declared}. Omit the key entirely for a ` + + 'single-package stack — `manifest` is retained, not replaced.', + ) as StackPackagesError; + err.code = 'INVALID_ARTIFACT_PACKAGES'; + err.status = 422; + throw err; +} + function strName(v: unknown): string | undefined { return typeof v === 'string' && v.length > 0 ? v : undefined; } diff --git a/packages/lint/src/validate-mapping-target-fields.test.ts b/packages/lint/src/validate-mapping-target-fields.test.ts index aca2e7a22c5..1dc4a5295dc 100644 --- a/packages/lint/src/validate-mapping-target-fields.test.ts +++ b/packages/lint/src/validate-mapping-target-fields.test.ts @@ -180,6 +180,39 @@ describe('validateMappingTargetFields', () => { expect(validateMappingTargetFields(stack)).toEqual([]); }); + it('CONTROL — `packages: undefined` (absent) stays silent — the only value this reader treats as absent', () => { + // `full_name`, not `sla_tier`: `sla_tier` resolves via the STACK's own + // top-level `objectExtensions` (the test above, :173) — `region` is the + // one that needs a package (:174). Either way, with `packages` genuinely + // absent this control needs a field `contact` declares on its own, so a + // real finding can't masquerade as the reader silently accepting the + // shape. + expect(validateMappingTargetFields({ + objects: [contact], + packages: undefined, + mappings: [mapping([{ source: 'Name', target: 'full_name' }])], + })).toEqual([]); + }); + + // [#20206, ruling A on #15293 `5634034754`] This reader was added by #20208 + // after the ruling's own site census (`origin/main` `1c8b320`) — a fifth + // copy of the same `recordsOf(stack.packages)` fall-through the ruling + // closes elsewhere in this package. A PRESENT non-array `packages` is + // malformed, not absent; only `undefined` stays silent. `null` joins this + // set in rework round 1 (ruling A on #19926, `5805260775`): it is present, + // not absent. A keyed object (the shape `recordsOf` read as a map) joins in + // rework round 3, alongside the explicit `undefined` control above, so this + // validator pins the same shape classes the other two validators do. + it('refuses a PRESENT non-array `packages` instead of silently ignoring it', () => { + for (const packages of [{}, 0, 'x', null, { a: { manifest: {} } }]) { + expect(() => validateMappingTargetFields({ + objects: [contact], + packages, + mappings: [mapping([{ source: 'Tier', target: 'sla_tier' }])], + })).toThrow(expect.objectContaining({ code: 'INVALID_ARTIFACT_PACKAGES', status: 422 })); + } + }); + it('stays silent on a mapping whose object this stack does not define (skip 1)', () => { expect(validateMappingTargetFields({ objects: [contact], diff --git a/packages/lint/src/validate-mapping-target-fields.ts b/packages/lint/src/validate-mapping-target-fields.ts index be251cecd01..0223db807e7 100644 --- a/packages/lint/src/validate-mapping-target-fields.ts +++ b/packages/lint/src/validate-mapping-target-fields.ts @@ -64,7 +64,7 @@ import { type ImportMappingTargetHead, } from '@objectstack/spec/data'; -import { listNames, recordsOf, suggestName } from './object-graph.js'; +import { listNames, packagesOf, recordsOf, suggestName } from './object-graph.js'; /** * A `fieldMapping[].target` the mapping cannot write on its object: it names no @@ -112,7 +112,7 @@ function extensionFieldsByTarget(stack: AnyRec): Map { } }; add(stack.objectExtensions); - for (const entry of recordsOf(stack.packages)) { + for (const entry of packagesOf(stack)) { if (isRec(entry.manifest)) add(entry.manifest.objectExtensions); } return byTarget; diff --git a/packages/lint/src/validate-object-references.test.ts b/packages/lint/src/validate-object-references.test.ts index 831c430ea0d..bfb494ab52b 100644 --- a/packages/lint/src/validate-object-references.test.ts +++ b/packages/lint/src/validate-object-references.test.ts @@ -297,10 +297,25 @@ describe('validateObjectReferences — artifact packages[] as resolution context expect(findings[0].path).toBe('objects[0].fields.account.reference'); }); - it('ignores a `packages` value that is not a list of entries', () => { - for (const packages of [null, 42, 'core']) { - const findings = validateObjectReferences(perPackageStack(ORDERS_BODY, packages)); - expect(findings.map((f) => f.path)).toEqual(['objects[0].fields.account.reference']); + it('CONTROL — `packages: undefined` (absent) stays silent — the only value this reader treats as absent', () => { + const findings = validateObjectReferences(perPackageStack(ORDERS_BODY, undefined)); + expect(findings.map((f) => f.path)).toEqual(['objects[0].fields.account.reference']); + }); + + // [#20206, ruling A on #15293 `5634034754`] Was "ignores a `packages` value + // that is not a list of entries" — `null`, `42` and `'core'` used to fall + // through `recordsOf` to `[]` and be silently treated as "no packages", + // exactly the fall-through the ruling closes: PRESENT but not an array is + // malformed, not absent, and every reader refuses it. `null` joins this set + // in rework round 1 (ruling A on #19926, `5805260775`): it is present, not + // absent, so it is no longer a control. `{}` and a keyed object join in + // rework round 2, so this validator pins the same shape classes `packagesOf` + // and the other two validators do. + it('refuses a PRESENT non-array `packages` instead of silently ignoring it', () => { + for (const packages of [null, 42, 'core', {}, { a: { manifest: {} } }]) { + expect(() => validateObjectReferences(perPackageStack(ORDERS_BODY, packages))).toThrow( + expect.objectContaining({ code: 'INVALID_ARTIFACT_PACKAGES', status: 422 }), + ); } }); }); diff --git a/packages/lint/src/validate-object-references.ts b/packages/lint/src/validate-object-references.ts index befd1c7517b..013e4df7c3f 100644 --- a/packages/lint/src/validate-object-references.ts +++ b/packages/lint/src/validate-object-references.ts @@ -81,7 +81,7 @@ import { } from '@objectstack/spec/system'; import { referenceCarrierOf, referenceTargetOf } from '@objectstack/spec/data'; -import { recordsOf, suggestName } from './object-graph.js'; +import { packagesOf, recordsOf, suggestName } from './object-graph.js'; /** Materialized once for the repeated edit-distance scans in `suggestName`. */ const PLATFORM_NAMES: readonly string[] = [...PLATFORM_PROVIDED_OBJECT_NAMES]; @@ -162,7 +162,7 @@ function isInterpolated(target: string): boolean { */ function artifactProvidedObjectNames(stack: AnyRec): string[] { const names: string[] = []; - for (const entry of recordsOf(stack.packages)) { + for (const entry of packagesOf(stack)) { const body = entry.manifest; if (!body || typeof body !== 'object' || Array.isArray(body)) continue; for (const obj of recordsOf((body as AnyRec).objects)) { diff --git a/packages/lint/src/validate-translation-references.test.ts b/packages/lint/src/validate-translation-references.test.ts index b03e448e85a..2cdb41262dd 100644 --- a/packages/lint/src/validate-translation-references.test.ts +++ b/packages/lint/src/validate-translation-references.test.ts @@ -528,6 +528,36 @@ describe('validateTranslationReferences — cross-package objects (§4 ladder)', }); }); +describe('validateTranslationReferences — a PRESENT non-array `packages` (#20206, ruling A on #15293 `5634034754`)', () => { + // `packages` is declared array-or-absent (ADR-0130 D4), never map-or-array. + // This rule reads it through the three internal carriers `object-graph.ts`'s + // `packagesOf` now serves (contributed nav, object extensions, and every + // artifact-provided collection) — all three now refuse instead of silently + // reading the malformed value as "no packages". `artifactProvidedRecords` + // ('objects') runs first inside `buildUniverse`, so that is the carrier this + // pin observes throwing; the read itself is pinned exhaustively, once, in + // `object-graph.test.ts` (the shared function all three now call). + // A bundle is required — `validateTranslationReferences` returns before ever + // calling `buildUniverse` (and therefore before reading `packages` at all) + // when `stack.translations` is empty, exactly like the exemptions below. + const oneBundle = [{ 'zh-CN': { objects: {} } }]; + + it('refuses instead of silently treating it as absent', () => { + for (const packages of [{}, 0, 'x', { a: { manifest: {} } }, null]) { + expect(() => validateTranslationReferences({ objects: [], translations: oneBundle, packages })).toThrow( + expect.objectContaining({ code: 'INVALID_ARTIFACT_PACKAGES', status: 422 }), + ); + } + }); + + // [ruling A on #19926, `5805260775`] `null` moved from the control above + // into the refusal set in rework round 1: it is present, not absent. + it('CONTROL — only an absent (`undefined`) `packages` stays silent', () => { + expect(validateTranslationReferences({ objects: [], translations: oneBundle })).toEqual([]); + expect(validateTranslationReferences({ objects: [], translations: oneBundle, packages: undefined })).toEqual([]); + }); +}); + describe('validateTranslationReferences — apps, dashboards, global actions', () => { const stack = { objects: [{ name: 'crm_lead', fields: { name: { type: 'text' } } }], diff --git a/packages/lint/src/validate-translation-references.ts b/packages/lint/src/validate-translation-references.ts index d6b2706ecbd..19a604da51a 100644 --- a/packages/lint/src/validate-translation-references.ts +++ b/packages/lint/src/validate-translation-references.ts @@ -167,7 +167,7 @@ import { expandViewContainer } from '@objectstack/spec'; import { hasPlatformObjectPrefix, isPlatformProvidedObjectName } from '@objectstack/spec/system'; import { walkFlowNodes } from './flow-walk.js'; -import { recordsOf, suggestName } from './object-graph.js'; +import { packagesOf, recordsOf, suggestName } from './object-graph.js'; import { walkPageComponents } from './page-walk.js'; import { SYSTEM_FIELDS } from './system-fields.js'; import { viewObjectName } from './view-walk.js'; @@ -750,7 +750,7 @@ function contributedNavItemsByApp(stack: AnyRec): Map { } }; add(isRec(stack.manifest) ? stack.manifest.navigationContributions : undefined); - for (const entry of recordsOf(stack.packages)) { + for (const entry of packagesOf(stack)) { const body = entry.manifest; if (!isRec(body)) continue; add(body.navigationContributions); @@ -845,7 +845,7 @@ function objectExtensionsByTarget(stack: AnyRec): Map { } }; add(stack.objectExtensions); - for (const entry of recordsOf(stack.packages)) { + for (const entry of packagesOf(stack)) { const body = entry.manifest; if (!isRec(body)) continue; add(body.objectExtensions); @@ -918,7 +918,7 @@ function objectExtensionsByTarget(stack: AnyRec): Map { */ function artifactProvidedRecords(stack: AnyRec, collection: string): AnyRec[] { const provided: AnyRec[] = []; - for (const entry of recordsOf(stack.packages)) { + for (const entry of packagesOf(stack)) { const body = entry.manifest; if (!isRec(body)) continue; provided.push(...recordsOf(body[collection])); diff --git a/packages/spec/src/api/error-code-ledger.zod.ts b/packages/spec/src/api/error-code-ledger.zod.ts index c495afc5229..1dba8559b53 100644 --- a/packages/spec/src/api/error-code-ledger.zod.ts +++ b/packages/spec/src/api/error-code-ledger.zod.ts @@ -1357,6 +1357,24 @@ export const ERROR_CODE_LEDGER = { 'STACK_COMPOSE_KEY_CONFLICT', // a single-valued top-level key is declared with different values by two stacks 'STACK_COMPOSE_OBJECT_CONFLICT', // the same object name is defined by more than one stack under the default `objectConflict: 'error'` ], + '@objectstack/lint': [ + // [#20206] `packagesOf` (`object-graph.ts`) refuses a present non-array + // `stack.packages` ({}, 0, 'x', a keyed object, `null`) — for every shape + // OTHER than `null`, with the SAME registered code `@objectstack/core`'s + // `resolveArtifactPackageOrder` already raises for the identical defect + // on an assembled artifact (ruling A on #15293 `5634034754`). For `null` + // (ruling A on #19926 `5805260775`), core raises this code for `null` + // too (#19926) — deliberately reused either way, never minted, so an + // author sees one code regardless of which reader catches the malformed + // shape first. `door: 'none'`, the #16449 reading: `packages/lint`'s + // rules are pure `(stack) => Finding[]` functions, reachable only from + // `os lint` (`packages/cli`) — `os validate` / `os build` refuse a + // malformed `packages` earlier, at `ObjectStackDefinitionSchema.safeParse`, + // before these rules ever run — never through an HTTP boundary either + // way, the same posture `@objectstack/spec`'s own `STACK_*` rows above + // record. + 'INVALID_ARTIFACT_PACKAGES', + ], } as const satisfies Record; /** A code registered by at least one package (deduped union of the ledger). */