diff --git a/.changeset/15293-non-array-packages-refusal.md b/.changeset/15293-non-array-packages-refusal.md index e75f7492f6f..7c948792c29 100644 --- a/.changeset/15293-non-array-packages-refusal.md +++ b/.changeset/15293-non-array-packages-refusal.md @@ -9,5 +9,5 @@ Clause-②: no `packages` is declared as an array of package entries (`ObjectStackDefinitionSchema.packages: z.array(ArtifactPackageSchema).optional()`), and the rule is now written down once, beside `AssembledPackageBodySchema` in `@objectstack/spec`: an absent `packages` means a single-package artifact, and any other non-array value is malformed and refused. `resolveArtifactPackageOrder` in `@objectstack/core` already refused it, and so did the i18n detector in `@objectstack/plugin-dev` and the default-permission-set reader in `@objectstack/plugin-security`. - **What changes**: `AppPlugin` reads its collections in `start()`, and `start()` now raises the same refusal `init()` already raised through the kernel's `manifest` service. Under `os dev`, `DevPlugin`'s child-`start()` loop logs it on its `error` line, where before the app started on its top-level collections alone. `createStandaloneStack` now refuses such an artifact while it builds the stack. Before, the refusal came later, when the app registered with the `manifest` service. `loadArtifactBundle`'s runtime-module merge reports it through its existing `warn` line and skips the merge, as it already does for a malformed `packages[]` entry. `resolveProjectDatabaseUrl` no longer reads a default datasource out of such an artifact: it declines, as it already does for any artifact it cannot read, and moves on to the next rung (the unified default database). The boot that loads the artifact then refuses it. -- **What does not change**: an absent `packages`, and `packages: null`, still return the caller's own object by identity. A well-formed `packages[]` resolves exactly as before. +- **What does not change**: an absent `packages` still returns the caller's own object by identity. A well-formed `packages[]` resolves exactly as before. `packages: null` is not absent: it is malformed, and it is refused the same way (#19926). - **Fix**: remove the `packages` key for a single-package artifact, or make it an array of `{ manifest: … }` entries. diff --git a/.changeset/19926-packages-null-refused.md b/.changeset/19926-packages-null-refused.md new file mode 100644 index 00000000000..e5198808560 --- /dev/null +++ b/.changeset/19926-packages-null-refused.md @@ -0,0 +1,24 @@ +--- +'@objectstack/core': minor +'@objectstack/runtime': minor +'@objectstack/plugin-dev': minor +'@objectstack/plugin-security': minor +--- + +fix(core,runtime,plugin-dev,plugin-security): a release artifact whose `packages` is `null` is refused as malformed, never read as absent (#19926) + +Clause-②: no (narrowing) + + + +**BREAKING** — an accept-set narrowing on a value the schema already refuses, shipped as `minor` under the launch-window convention (`check-changeset-no-major` refuses `major` until GA; breaking-ness is carried by this banner and the ADR-0087 disposition above, not by the level). + +`ObjectStackDefinitionSchema.packages` is `z.array(ArtifactPackageSchema).optional()`, and `.optional()` admits `undefined`, not `null`. The schema refused `packages: null` (`invalid_type`), and `composeStacks` refused it with two or more inputs (`STACK_SCHEMA_INVALID`, `status: 422`). The runtime readers below read it as absent instead: a single-package artifact whose own top level is the one package body. Those readers now follow the declaration. An absent `packages` is `undefined` and nothing else; `null` is one of the present, non-array values the rule beside `AssembledPackageBodySchema` calls malformed, like `{}`, `0` or `'x'`, and it is refused with the same envelope: `INVALID_ARTIFACT_PACKAGES`, `status: 422`. No error code is added. + +- **`@objectstack/core`**: `resolveArtifactPackageOrder` refuses `packages: null` where it returned `[artifact]`. The refusal message names the value `null`, not `object`. The resolver's callers that hand it the whole artifact raise the refusal: the kernel `manifest` service's `register()` (`ObjectQLPlugin`) and `@objectstack/verify`'s collection reader for a collection the stack's top level does not carry. +- **`@objectstack/runtime`**: `resolveArtifactCollections` drops `null` from its absent branch, so `AppPlugin`, `createStandaloneStack`, `loadArtifactBundle`'s runtime-module merge and `resolveProjectDatabaseUrl` answer a `packages: null` artifact exactly as they already answer `packages: {}`. `carriedPackageIds`, and `resolveArtifactGrantBinding` for an artifact whose `grantedPermissions` is a record, read the package list through the core resolver and raise its refusal too. +- **`@objectstack/plugin-dev`**: the i18n detector's private absent guard moves in lockstep with the resolver's absent branch, so `devI18nPluginOptions` reaches the resolver and raises its refusal when the `i18n` config (on the stack or its `manifest`), a non-empty `manifest.translations` and a non-empty top-level `translations` do not answer first. `DevPlugin` keeps its posture: it reports the metadata defect on its `error` line and boots on the in-memory i18n fallback. +- **`@objectstack/plugin-security`**: `appSecurityPluginOptions` has no guard of its own and raises the resolver's refusal for `packages: null`. +- **What does not change**: the schema; an absent `packages` (no key, or an explicit `undefined`), which still returns the caller's own object by identity; a well-formed `packages[]`; and `composeStacks` with a single input, which still returns that input by identity. + +No in-repo producer writes `packages: null`, and `os build` and `os validate` refuse it at the schema before any reader runs. For a single-package artifact, leave the `packages` key out. diff --git a/packages/core/src/artifact-packages.test.ts b/packages/core/src/artifact-packages.test.ts new file mode 100644 index 00000000000..dee16ed2e40 --- /dev/null +++ b/packages/core/src/artifact-packages.test.ts @@ -0,0 +1,69 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * `resolveArtifactPackageOrder`'s ABSENT branch is `undefined` only (#19926, + * ruling A). + * + * `ObjectStackDefinitionSchema.packages` is `z.array(ArtifactPackageSchema) + * .optional()`, and `.optional()` admits `undefined`, not `null`. So `null` is + * a present, non-array `packages`: malformed, and refused with the same + * envelope as `{}`, `0` or `'x'`. The rule is stated once, beside + * `AssembledPackageBodySchema` (`@objectstack/spec`, `stack.zod.ts`). + * + * The resolver's wider behaviour (ordering, the entry gate, duplicates) is + * pinned where its load path is, in `@objectstack/objectql`'s + * `artifact-load-path.test.ts`. This file pins the one branch every reader of + * `packages` inherits from here. + */ + +import { describe, it, expect } from 'vitest'; +import { resolveArtifactPackageOrder, type ArtifactPackageError } from './artifact-packages'; + +const manifest = { id: 'com.example.a', name: 'A', version: '1.0.0', type: 'app' }; + +function refusalOf(artifact: unknown): ArtifactPackageError | undefined { + try { + resolveArtifactPackageOrder(artifact); + return undefined; + } catch (err) { + return err as ArtifactPackageError; + } +} + +describe('resolveArtifactPackageOrder — `packages: null` is malformed, not absent', () => { + it('refuses `packages: null` with INVALID_ARTIFACT_PACKAGES / 422', () => { + const refused = refusalOf({ manifest, packages: null }); + expect(refused).toBeInstanceOf(Error); + expect(refused?.code).toBe('INVALID_ARTIFACT_PACKAGES'); + expect(refused?.status).toBe(422); + }); + + it.each([ + ['{}', {}], + ['0', 0], + ["'x'", 'x'], + ])('refuses `packages: %s` with the same envelope — `null` is one of these, not a fourth case', (_label, packages) => { + const refused = refusalOf({ manifest, packages }); + expect(refused?.code).toBe('INVALID_ARTIFACT_PACKAGES'); + expect(refused?.status).toBe(422); + }); + + it('control: an ABSENT `packages` (no key, or an explicit `undefined`) is the single-package branch, returned by identity', () => { + const noKey = { manifest }; + const explicitUndefined = { manifest, packages: undefined }; + for (const artifact of [noKey, explicitUndefined]) { + expect(refusalOf(artifact)).toBeUndefined(); + const resolved = resolveArtifactPackageOrder(artifact); + expect(resolved).toHaveLength(1); + expect(resolved[0]).toBe(artifact); + } + }); + + it('control: an ARRAY `packages` resolves to its bodies, by reference', () => { + const body = { ...manifest }; + const resolved = resolveArtifactPackageOrder({ packages: [{ manifest: body }] }); + expect(resolved).toHaveLength(1); + expect(resolved[0]).toBe(body); + expect(resolveArtifactPackageOrder({ packages: [] })).toEqual([]); + }); +}); diff --git a/packages/core/src/artifact-packages.ts b/packages/core/src/artifact-packages.ts index ac5eed9d162..aeebb900c8f 100644 --- a/packages/core/src/artifact-packages.ts +++ b/packages/core/src/artifact-packages.ts @@ -35,7 +35,9 @@ * - `packages` absent → treat `manifest` (singular) as a **single-element list**. * * A `packages` that is present but is not an array takes neither branch. It is - * refused here as `INVALID_ARTIFACT_PACKAGES`. The rule is stated once, + * refused here as `INVALID_ARTIFACT_PACKAGES`. `null` is one of those values: + * the key is declared `.optional()`, which admits `undefined` and not `null`, + * so ABSENT means `undefined` and nothing else. The rule is stated once, * beside `AssembledPackageBodySchema` (`@objectstack/spec`, `stack.zod.ts`). * * The second branch is not a convenience: it is the term ADR-0130's whole @@ -194,7 +196,7 @@ interface ArtifactPackageNode extends OrderablePlugin { * @returns The manifest bodies to register, in the order to register them. * @throws An ADR-0112 envelope (`code` + `status: 422`): * `INVALID_ARTIFACT_PACKAGES` for a `packages` that is present but is not an - * array, `INVALID_ARTIFACT_PACKAGE_ENTRY` for a malformed entry, and + * array (`null` included), `INVALID_ARTIFACT_PACKAGE_ENTRY` for a malformed entry, and * `DUPLICATE_ARTIFACT_PACKAGE` for a duplicate package id. Also * `resolvePluginOrder`'s own error for a cycle. */ @@ -205,7 +207,12 @@ export function resolveArtifactPackageOrder(artifact: unknown): unknown[] { // the caller's own object IS that package's manifest body. Returned by // reference, unvalidated and unrewritten — this is the path every artifact // built to date takes, and D7 pins that it did not move. - if (declared === undefined || declared === null) return [artifact]; + // + // ⛔ `undefined` ONLY. `null` is present, not absent: the schema's + // `.optional()` refuses it, so reading it as absent here would answer for an + // artifact the declaration calls malformed (#19926). It falls to the refusal + // below with every other non-array value. + if (declared === undefined) return [artifact]; // Present but not an array: malformed, never absent. The rule is stated // once, beside `AssembledPackageBodySchema`. @@ -214,7 +221,9 @@ export function resolveArtifactPackageOrder(artifact: unknown): unknown[] { 'INVALID_ARTIFACT_PACKAGES', 'A release artifact\'s `packages` must be an array of package entries ' + '(ADR-0130 D4, `ArtifactPackageEntrySchema`), but this artifact carries ' - + `\`packages\` of type ${typeof declared}. Omit the key entirely for a ` + // `typeof null` is `'object'`, which would name a `{}` the author never + // wrote; `null` is named as itself. + + `\`packages\` of type ${declared === null ? 'null' : typeof declared}. Omit the key entirely for a ` + 'single-package artifact — `manifest` is retained, not replaced.', ); } diff --git a/packages/plugins/plugin-dev/src/dev-i18n-packages-reader.test.ts b/packages/plugins/plugin-dev/src/dev-i18n-packages-reader.test.ts index 781e6f1fb2c..16f8870762c 100644 --- a/packages/plugins/plugin-dev/src/dev-i18n-packages-reader.test.ts +++ b/packages/plugins/plugin-dev/src/dev-i18n-packages-reader.test.ts @@ -44,7 +44,8 @@ import { describe, it, expect, vi } from 'vitest'; import { composeStacks, defineStack, type ObjectStackDefinition } from '@objectstack/spec'; -import { devI18nPluginOptions } from './dev-i18n'; +import { resolveArtifactPackageOrder } from '@objectstack/core'; +import { devI18nPluginOptions, stackDeclaresTranslations } from './dev-i18n'; import { DevPlugin } from './dev-plugin'; const absent = (name: string): Error => @@ -342,7 +343,9 @@ describe('#15232 — DevPlugin i18n auto-detect over a multi-package stack', () // A `packages` that is present but is not an array is MALFORMED, not absent // (the rule beside `AssembledPackageBodySchema`). This reader's private guard - // may decide only the absent branch, so these three reach the resolver. + // may decide only the absent branch, so these four reach the resolver. + // `null` is one of them since #19926 moved the guard and the resolver's + // absent branch together. const refusalOf = (stack: unknown): (Error & { code?: string; status?: number }) | undefined => { try { devI18nPluginOptions(stack); @@ -353,6 +356,7 @@ describe('#15232 — DevPlugin i18n auto-detect over a multi-package stack', () }; it.each([ + ['null', null], ['{}', {}], ['0', 0], ["'x'", 'x'], @@ -370,9 +374,10 @@ describe('#15232 — DevPlugin i18n auto-detect over a multi-package stack', () expect(refusalOf(optionBProject())).toBeUndefined(); expect(devI18nPluginOptions(optionBProject())).toEqual({ defaultLocale: undefined, fallbackLocale: 'en' }); - // Absent, explicitly `undefined`, and `null`: the guard's one decision. + // Absent and explicitly `undefined`: the guard's one decision. `null` is + // not here — it is a row of the refusal table above. const manifest = { id: CORE_ID, name: 'x', version: '1.0.0', type: 'app' }; - for (const absent of [{}, { packages: undefined }, { packages: null }]) { + for (const absent of [{}, { packages: undefined }]) { expect(refusalOf({ manifest, ...absent })).toBeUndefined(); expect(devI18nPluginOptions({ manifest, ...absent })).toBeUndefined(); expect(devI18nPluginOptions({ manifest, ...absent, translations: [{ en: {} }] })) @@ -380,6 +385,38 @@ describe('#15232 — DevPlugin i18n auto-detect over a multi-package stack', () } }); + it('LOCKSTEP — the private guard and `resolveArtifactPackageOrder` agree on `packages: null` and on an absent key', () => { + // The guard is bound to the resolver's absent branch: it may answer + // "absent" for exactly the values the resolver answers `[stack]` for, and + // must hand every other value to it. Asked of both directly, so a guard + // that drifted back to `undefined || null` goes red here even while the + // resolver refuses — the pair measured nothing before #19926. + const envelopeOf = (fn: () => unknown): { code?: unknown; status?: unknown } | undefined => { + try { + fn(); + return undefined; + } catch (err) { + const { code, status } = err as { code?: unknown; status?: unknown }; + return { code, status }; + } + }; + const manifest = { id: CORE_ID, name: 'x', version: '1.0.0', type: 'app' }; + + const nullStack = { manifest, packages: null }; + const fromResolver = envelopeOf(() => resolveArtifactPackageOrder(nullStack)); + const fromGuard = envelopeOf(() => stackDeclaresTranslations(nullStack)); + expect(fromResolver).toEqual({ code: 'INVALID_ARTIFACT_PACKAGES', status: 422 }); + expect(fromGuard).toEqual(fromResolver); + + // Control: an absent key is absent to both, and neither throws. + for (const absent of [{ manifest }, { manifest, packages: undefined }]) { + expect(envelopeOf(() => resolveArtifactPackageOrder(absent))).toBeUndefined(); + expect(resolveArtifactPackageOrder(absent)).toEqual([absent]); + expect(envelopeOf(() => stackDeclaresTranslations(absent))).toBeUndefined(); + expect(stackDeclaresTranslations(absent)).toBe(false); + } + }); + // ── What the developer actually gets: the SERVICE ───────────────────────── const bootWith = async (stack: Record | undefined) => { diff --git a/packages/plugins/plugin-dev/src/dev-i18n.ts b/packages/plugins/plugin-dev/src/dev-i18n.ts index bfa4c0cb0e2..6703956bca9 100644 --- a/packages/plugins/plugin-dev/src/dev-i18n.ts +++ b/packages/plugins/plugin-dev/src/dev-i18n.ts @@ -109,11 +109,13 @@ const declaresTranslationArray = (body: unknown): boolean => { * reader to exactly the old path, and it goes red without the guard. * * ⛔ So the guard is allowed to decide ONE thing: the ABSENT branch, spelled - * exactly as the resolver's own absent branch (`undefined` / `null`). Every - * other value goes to the resolver, so a present non-array `packages` (`{}`, - * `0`, `'x'`) is REFUSED as `INVALID_ARTIFACT_PACKAGES`. ⛔ Never widen it to - * `Array.isArray`: that is the silent fall-through the rule above forbids. If - * the resolver's absent branch ever changes, this line changes with it. + * exactly as the resolver's own absent branch (`undefined`, and nothing else). + * Every other value goes to the resolver, so a present non-array `packages` + * (`null`, `{}`, `0`, `'x'`) is REFUSED as `INVALID_ARTIFACT_PACKAGES`. ⛔ Never + * widen it to `Array.isArray`: that is the silent fall-through the rule above + * forbids. If the resolver's absent branch ever changes, this line changes with + * it — it did once, when `null` stopped being absent (#19926), and the two moved + * in one change. * * ## A malformed `packages[]` is refused, not skipped * @@ -163,7 +165,7 @@ const declaresTranslationArray = (body: unknown): boolean => { * array. * @throws An ADR-0112 envelope (`Error & { code, status: 422 }`) from * `resolveArtifactPackageOrder` when `packages` is present but not loadable: - * `INVALID_ARTIFACT_PACKAGES` (not an array), `INVALID_ARTIFACT_PACKAGE_ENTRY` + * `INVALID_ARTIFACT_PACKAGES` (not an array, `null` included), `INVALID_ARTIFACT_PACKAGE_ENTRY` * (an entry that is not `{ manifest: … }`, a body carrying authoring-time * globs where definitions belong, or a manifest with no usable id) or * `DUPLICATE_ARTIFACT_PACKAGE`. @@ -179,7 +181,7 @@ export function stackDeclaresTranslations(stack: unknown): boolean { if (declaresTranslationArray(stack)) return true; const packages = asBag(stack)?.packages; - if (packages === undefined || packages === null) return false; + if (packages === undefined) return false; for (const body of resolveArtifactPackageOrder(stack)) { if (declaresTranslationArray(body)) return true; diff --git a/packages/plugins/plugin-security/src/app-default-permission-set.test.ts b/packages/plugins/plugin-security/src/app-default-permission-set.test.ts index 430b55e4629..e06540574c8 100644 --- a/packages/plugins/plugin-security/src/app-default-permission-set.test.ts +++ b/packages/plugins/plugin-security/src/app-default-permission-set.test.ts @@ -341,8 +341,10 @@ describe('appSecurityPluginOptions over `packages[]` (ADR-0130 D4, #15007)', () // A `packages` that is present but is not an array is MALFORMED, not absent // (the rule beside `AssembledPackageBodySchema`). This reader keeps no - // `packages` guard of its own, so the refusal is the resolver's. + // `packages` guard of its own, so the refusal is the resolver's — `null` + // included, since #19926 took `null` out of the resolver's absent branch. it.each([ + ['null', null], ['{}', {}], ['0', 0], ["'x'", 'x'], @@ -359,9 +361,10 @@ describe('appSecurityPluginOptions over `packages[]` (ADR-0130 D4, #15007)', () expect(refusalOf({ packages: [wellFormed] })).toEqual({}); expect(appSecurityPluginOptions({ packages: [wellFormed] })).toEqual({ fallbackPermissionSet: CORE_PROFILE }); - // Absent, explicitly `undefined`, and `null`: all three read the top level - // exactly as before the private guard was dropped. - for (const absent of [{}, { packages: undefined }, { packages: null }]) { + // Absent and explicitly `undefined`: both read the top level exactly as + // before the private guard was dropped. `null` is a row of the refusal + // table above, not an absent key. + for (const absent of [{}, { packages: undefined }]) { expect(refusalOf({ ...absent, permissions: [permissionSet('top')] })).toEqual({}); expect(appSecurityPluginOptions({ ...absent, permissions: [permissionSet('top')] })) .toEqual({ fallbackPermissionSet: 'top' }); diff --git a/packages/plugins/plugin-security/src/app-default-permission-set.ts b/packages/plugins/plugin-security/src/app-default-permission-set.ts index 0fd7b6ea30a..ddc3f615109 100644 --- a/packages/plugins/plugin-security/src/app-default-permission-set.ts +++ b/packages/plugins/plugin-security/src/app-default-permission-set.ts @@ -172,8 +172,9 @@ export function appDefaultPermissionSetName(permissions: unknown): string | unde * ⛔ This reader has no `packages` guard of its own. Which `packages` values * are absent and which are refused is answered by `resolveArtifactPackageOrder` * alone. The rule is stated once, beside `AssembledPackageBodySchema` - * (`@objectstack/spec`, `stack.zod.ts`). A private `undefined` / `null` check - * here would be a second spelling of that answer. + * (`@objectstack/spec`, `stack.zod.ts`). A private `undefined` check here + * would be a second spelling of that answer. So `packages: null` is refused + * here exactly when the resolver refuses it, with its envelope. * * ## One thing it deliberately does NOT do * diff --git a/packages/runtime/src/artifact-collections.test.ts b/packages/runtime/src/artifact-collections.test.ts index 1b19473c6d4..44a443f6c2f 100644 --- a/packages/runtime/src/artifact-collections.test.ts +++ b/packages/runtime/src/artifact-collections.test.ts @@ -90,21 +90,20 @@ describe('resolveArtifactCollections', () => { expect(resolveArtifactCollections(null)).toBe(null); expect(resolveArtifactCollections(undefined)).toBe(undefined); expect(resolveArtifactCollections('not an object')).toBe('not an object'); - // An explicit `undefined`, and `null`, read as absent too. `null` is read - // this way by every reader today; the schema's `.optional()` refuses it, - // and that disagreement is recorded beside `AssembledPackageBodySchema` - // rather than decided here. + // An explicit `undefined` reads as absent too. `null` does NOT: the + // schema's `.optional()` admits `undefined` only, so `null` is a present + // non-array `packages` and is refused below (#19926, ruling A). const explicitUndefined = { packages: undefined, objects: [obj('o')] }; expect(resolveArtifactCollections(explicitUndefined)).toBe(explicitUndefined); - const nullPackages = { packages: null, objects: [obj('o')] }; - expect(resolveArtifactCollections(nullPackages)).toBe(nullPackages); }); // A `packages` that is present but is not an array is MALFORMED, not absent // (the rule beside `AssembledPackageBodySchema`). This reader used to hand // such an artifact back by identity, answering about its top level while - // the loader refused the same bytes. + // the loader refused the same bytes. `null` is one of these rows: it was + // handed back by identity until #19926 aligned this guard to the schema. it.each([ + ['null', null], ['{}', {}], ['0', 0], ["'x'", 'x'], diff --git a/packages/runtime/src/artifact-collections.ts b/packages/runtime/src/artifact-collections.ts index 5351f76d6c7..ff7ce1e6f0e 100644 --- a/packages/runtime/src/artifact-collections.ts +++ b/packages/runtime/src/artifact-collections.ts @@ -428,12 +428,13 @@ function mergeCollection(key: string, top: unknown, fromBodies: readonly Contrib * branch is an identity function on purpose: it is the only way to say "this * change cannot have moved the shape that ships today" rather than to hope so. * A `packages` that is present but is not an array is not absent, so it does - * NOT take that branch. It is malformed, and it reaches the refusal below. The - * rule is stated once, beside `AssembledPackageBodySchema` - * (`packages/spec/src/stack.zod.ts`). + * NOT take that branch. It is malformed, and it reaches the refusal below. + * `null` is such a value: ABSENT means `undefined` only, because the key's + * `.optional()` admits `undefined` and not `null`. The rule is stated once, + * beside `AssembledPackageBodySchema` (`packages/spec/src/stack.zod.ts`). * * @throws The ADR-0112 refusal `resolveArtifactPackageOrder` raises for a - * `packages` that is not an array (`INVALID_ARTIFACT_PACKAGES`), a malformed + * `packages` that is not an array, `null` included (`INVALID_ARTIFACT_PACKAGES`), a malformed * `packages[]` entry, a package with no usable id, or a duplicate package. * It is the same refusal `ObjectQLPlugin`'s `manifest` service already * raises on the same artifact during boot. Resolving collections out of an @@ -455,11 +456,13 @@ function mergeCollection(key: string, top: unknown, fromBodies: readonly Contrib */ export function resolveArtifactCollections(artifact: T): T { if (artifact === null || typeof artifact !== 'object') return artifact; - // ABSENT only. A present non-array `packages` falls to the refusal in + // ABSENT only, and absent is `undefined` only. A present non-array + // `packages` — `null` included — falls to the refusal in // `resolveArtifactPackageOrder` below; see the docblock for where the rule - // lives. `null` is read as absent, as the other readers read it. + // lives. This guard is spelled exactly as the resolver's own absent branch, + // so the two cannot disagree about which artifacts they answer for. const declared = (artifact as { packages?: unknown }).packages; - if (declared === undefined || declared === null) return artifact; + if (declared === undefined) return artifact; const bodies = resolveArtifactPackageOrder(artifact) as Array | null | undefined>; // `resolveArtifactPackageOrder` has already refused any entry whose manifest diff --git a/packages/spec/src/stack-artifact-packages.test.ts b/packages/spec/src/stack-artifact-packages.test.ts index defb91d13b0..d933932154c 100644 --- a/packages/spec/src/stack-artifact-packages.test.ts +++ b/packages/spec/src/stack-artifact-packages.test.ts @@ -250,6 +250,21 @@ describe('ADR-0130 D4 — each `packages` entry is an OBJECT wrapping its manife expect(issueAt(result, ['packages'])?.code).toBe('invalid_type'); }); + it('refuses `packages: null` — `.optional()` admits `undefined`, not `null` (#19926)', () => { + // `null` is a present, non-array `packages`: malformed, not absent. Every + // reader of the key refuses it too (`INVALID_ARTIFACT_PACKAGES`); the rule + // is stated once, beside `AssembledPackageBodySchema`. + const result = parse({ ...singleManifestArtifact(), packages: null }); + + expect(result.success).toBe(false); + expect(issueAt(result, ['packages'])?.code).toBe('invalid_type'); + + // Control: the same artifact with `packages` absent, or explicitly + // `undefined`, parses — so the refusal above is about `null` alone. + expect(parse(singleManifestArtifact()).success).toBe(true); + expect(parse({ ...singleManifestArtifact(), packages: undefined }).success).toBe(true); + }); + it('refuses TODAY\'s runtime the future `{ ref, integrity }` segment — cleanly', () => { // ⚠️ DELIBERATE, and it is the forward half of the reservation: an older // runtime must refuse a newer artifact rather than mis-parse it into a @@ -307,6 +322,36 @@ describe('ADR-0130 D4 — `packages` has a declared composition rule', () => { ]); }); + it('refuses `packages: null` on any input when composing two or more stacks (#19926)', () => { + // The concat pass skips `undefined` alone: `null` is malformed, not + // absent, and is refused with the strict parse's own envelope rather than + // composed as if the stack declared no packages. `strict: false` is the + // door that lets `null` reach composition at all — the strict parse + // refuses it first (the pin above). + const withNull = () => raw({ manifest: crmManifest, packages: null }); + const control = () => raw({ manifest: cpqManifest, packages: [{ manifest: cpqManifest }] }); + + for (const stacks of [[withNull(), control()], [control(), withNull()]]) { + let refused: (Error & { code?: string; status?: number; issues?: { path?: unknown[] }[] }) | undefined; + try { + composeStacks(stacks); + } catch (err) { + refused = err as typeof refused; + } + expect(refused).toBeInstanceOf(Error); + expect(refused?.code).toBe('STACK_SCHEMA_INVALID'); + expect(refused?.status).toBe(422); + expect(refused?.issues?.[0]?.path).toEqual(['packages']); + } + + // Control: the same composition with `packages` absent on that stack + // composes, carrying the other stack's entries. + const composed = composeStacks([raw({ manifest: crmManifest }), control()]) as unknown as { + packages: { manifest: { id: string } }[]; + }; + expect(composed.packages.map((p) => p.manifest.id)).toEqual(['com.example.crm.cpq']); + }); + it('does not warn about an undeclared composition rule (#5005 rule 3)', () => { composeStacks([ raw({ manifest: crmManifest, packages: [{ manifest: crmManifest }] }), diff --git a/packages/spec/src/stack.zod.ts b/packages/spec/src/stack.zod.ts index dcdfef43bf3..37e688387b5 100644 --- a/packages/spec/src/stack.zod.ts +++ b/packages/spec/src/stack.zod.ts @@ -1253,17 +1253,20 @@ function assembledPackageBodyShape(): Pick