Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .changeset/15293-non-array-packages-refusal.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
---
"@objectstack/runtime": patch
---
Expand All @@ -9,5 +9,5 @@
`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.
24 changes: 24 additions & 0 deletions .changeset/19926-packages-null-refused.md
Original file line number Diff line number Diff line change
@@ -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)

<!-- adr-0087: not-required (no-migration-prescription) Nothing authorable moves: no spec key, Zod schema, export, config field or stored metadata shape is added, removed, renamed or re-spelled. ObjectStackDefinitionSchema already refused packages null, and so did composeStacks with two or more inputs, so an authored stack that passed its schema reads exactly as before; what narrows is the runtime readers' behaviour on an artifact that bypassed that parse, and objectstack migrate meta has no document to rewrite for it. -->

**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.
69 changes: 69 additions & 0 deletions packages/core/src/artifact-packages.test.ts
Original file line number Diff line number Diff line change
@@ -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([]);
});
});
17 changes: 13 additions & 4 deletions packages/core/src/artifact-packages.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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.
*/
Expand All @@ -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`.
Expand All @@ -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.',
);
}
Expand Down
45 changes: 41 additions & 4 deletions packages/plugins/plugin-dev/src/dev-i18n-packages-reader.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 =>
Expand Down Expand Up @@ -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);
Expand All @@ -353,6 +356,7 @@ describe('#15232 — DevPlugin i18n auto-detect over a multi-package stack', ()
};

it.each([
['null', null],
['{}', {}],
['0', 0],
["'x'", 'x'],
Expand All @@ -370,16 +374,49 @@ 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: {} }] }))
.toEqual({ defaultLocale: undefined, fallbackLocale: 'en' });
}
});

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<string, unknown> | undefined) => {
Expand Down
16 changes: 9 additions & 7 deletions packages/plugins/plugin-dev/src/dev-i18n.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
*
Expand Down Expand Up @@ -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`.
Expand All @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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'],
Expand All @@ -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' });
Expand Down
Loading
Loading