Skip to content

Commit fdeeea0

Browse files
fix(runtime): refuse a non-array packages in resolveArtifactCollections (#19924)
Fixes #15293 Clause-②: no <sub>Rewritten short by the `domain:spec#5` seat (2026-09-23T20:02Z). The dev report is on #15293 (`5801982470`); the earlier long body is in the edit history.</sub> Ruling A (`5634034754`): a release artifact's `packages` that is present but is not an array (`{}`, `0`, `'x'`) is malformed, not absent, and every reader refuses it. `ObjectStackDefinitionSchema` already declares `packages: z.array(ArtifactPackageSchema).optional()`, so this narrows an accept set back to the declaration. ## What changed - **runtime** (the behaviour change): `resolveArtifactCollections` used to return the artifact for any non-array `packages`. It now treats only `undefined` / `null` as absent. Anything else reaches `resolveArtifactPackageOrder`, which refuses with `INVALID_ARTIFACT_PACKAGES` (422). - **spec**: the rule is stated once, beside `AssembledPackageBodySchema`. The docblocks in core and the two plugin readers point at it. - **plugin-security**: its private guard is dropped. Dropping it is behaviour-equal on the dev's 29-input differential. - **plugin-dev**: its private guard is kept, and the reason is written at the site. Dropping it turns an existing pin red. - **Pins**: `{}`, `0` and `'x'` at all three call sites, with lit controls. Reverting the runtime line turns the three runtime pins red. - **changeset**: `@objectstack/runtime` patch. ## Not in this PR - The `@objectstack/cli` readers still return nothing for a non-array `packages`: #19925. - `packages: null` is refused by the schema but read as absent by every reader: #19926. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01Sfe5YjBLwB9J3y8fvm2xq1 --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent ae0c90c commit fdeeea0

10 files changed

Lines changed: 221 additions & 39 deletions

File tree

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
---
2+
"@objectstack/runtime": patch
3+
---
4+
5+
A release artifact whose `packages` is present but is not an array (`{}`, `0`, `'x'`) is now refused by the runtime's collection reader too, as `INVALID_ARTIFACT_PACKAGES` (ADR-0112, `status: 422`) (#15293).
6+
7+
Clause-②: no
8+
9+
`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`.
10+
11+
- **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.
12+
- **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.
13+
- **Fix**: remove the `packages` key for a single-package artifact, or make it an array of `{ manifest: … }` entries.

‎content/docs/plugins/packages.mdx‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -400,6 +400,9 @@ metadata it names.
400400
one step later, when the app's own `init()` hands the stack to the kernel's
401401
`manifest` service and its package list is parsed; it reports as
402402
`INVALID_ARTIFACT_PACKAGE_ENTRY` (422) on the `error` line naming the app plugin.
403+
A `packages` value that is not an array at all, such as `{}`, takes the same
404+
branch and reports as `INVALID_ARTIFACT_PACKAGES` (422). It is malformed, not
405+
absent: for a single-package app, leave the key out.
403406

404407
The two differ at the production doors as well. The malformed `packages[]` fails
405408
the protocol schema, so both `os validate` and `os build` exit 1 on it. The missing

‎packages/core/src/artifact-packages.ts‎

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,10 @@
3434
* - `packages` present → iterate it.
3535
* - `packages` absent → treat `manifest` (singular) as a **single-element list**.
3636
*
37+
* A `packages` that is present but is not an array takes neither branch. It is
38+
* refused here as `INVALID_ARTIFACT_PACKAGES`. The rule is stated once,
39+
* beside `AssembledPackageBodySchema` (`@objectstack/spec`, `stack.zod.ts`).
40+
*
3741
* The second branch is not a convenience: it is the term ADR-0130's whole
3842
* compatibility claim rests on (D7 — an existing single-`manifest` artifact must
3943
* register bit-identically through this path). That is why this function returns
@@ -188,8 +192,11 @@ interface ArtifactPackageNode extends OrderablePlugin {
188192
* @param artifact - A release artifact (`{ packages: [...] }`), or a bare
189193
* manifest / single-`manifest` artifact — both shapes are read.
190194
* @returns The manifest bodies to register, in the order to register them.
191-
* @throws An ADR-0112 envelope (`code` + `status: 422`) for a malformed entry or
192-
* a duplicate package id, and `resolvePluginOrder`'s own error for a cycle.
195+
* @throws An ADR-0112 envelope (`code` + `status: 422`):
196+
* `INVALID_ARTIFACT_PACKAGES` for a `packages` that is present but is not an
197+
* array, `INVALID_ARTIFACT_PACKAGE_ENTRY` for a malformed entry, and
198+
* `DUPLICATE_ARTIFACT_PACKAGE` for a duplicate package id. Also
199+
* `resolvePluginOrder`'s own error for a cycle.
193200
*/
194201
export function resolveArtifactPackageOrder(artifact: unknown): unknown[] {
195202
const declared = (artifact as { packages?: unknown } | null | undefined)?.packages;
@@ -200,6 +207,8 @@ export function resolveArtifactPackageOrder(artifact: unknown): unknown[] {
200207
// built to date takes, and D7 pins that it did not move.
201208
if (declared === undefined || declared === null) return [artifact];
202209

210+
// Present but not an array: malformed, never absent. The rule is stated
211+
// once, beside `AssembledPackageBodySchema`.
203212
if (!Array.isArray(declared)) {
204213
throw refuse(
205214
'INVALID_ARTIFACT_PACKAGES',

‎packages/plugins/plugin-dev/src/dev-i18n-packages-reader.test.ts‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -340,6 +340,46 @@ describe('#15232 — DevPlugin i18n auto-detect over a multi-package stack', ()
340340
expect(caught?.message).toContain('packages[0]');
341341
});
342342

343+
// A `packages` that is present but is not an array is MALFORMED, not absent
344+
// (the rule beside `AssembledPackageBodySchema`). This reader's private guard
345+
// may decide only the absent branch, so these three reach the resolver.
346+
const refusalOf = (stack: unknown): (Error & { code?: string; status?: number }) | undefined => {
347+
try {
348+
devI18nPluginOptions(stack);
349+
return undefined;
350+
} catch (err) {
351+
return err as Error & { code?: string; status?: number };
352+
}
353+
};
354+
355+
it.each([
356+
['{}', {}],
357+
['0', 0],
358+
["'x'", 'x'],
359+
])('`packages: %s` is REFUSED with INVALID_ARTIFACT_PACKAGES, never read as absent', (_label, packages) => {
360+
// No `i18n` config and no top-level `translations`, so the question
361+
// reaches the package pass. See the next case for a stack that never does.
362+
const caught = refusalOf({ manifest: { id: CORE_ID, name: 'x', version: '1.0.0', type: 'app' }, packages });
363+
expect(caught?.code).toBe('INVALID_ARTIFACT_PACKAGES');
364+
expect(caught?.status).toBe(422);
365+
});
366+
367+
it('lit controls for the rows above: a well-formed `packages[]` resolves, and an absent key takes the single-package branch', () => {
368+
// Without these, an instrument that always threw would pin the three rows
369+
// above just as green.
370+
expect(refusalOf(optionBProject())).toBeUndefined();
371+
expect(devI18nPluginOptions(optionBProject())).toEqual({ defaultLocale: undefined, fallbackLocale: 'en' });
372+
373+
// Absent, explicitly `undefined`, and `null`: the guard's one decision.
374+
const manifest = { id: CORE_ID, name: 'x', version: '1.0.0', type: 'app' };
375+
for (const absent of [{}, { packages: undefined }, { packages: null }]) {
376+
expect(refusalOf({ manifest, ...absent })).toBeUndefined();
377+
expect(devI18nPluginOptions({ manifest, ...absent })).toBeUndefined();
378+
expect(devI18nPluginOptions({ manifest, ...absent, translations: [{ en: {} }] }))
379+
.toEqual({ defaultLocale: undefined, fallbackLocale: 'en' });
380+
}
381+
});
382+
343383
// ── What the developer actually gets: the SERVICE ─────────────────────────
344384

345385
const bootWith = async (stack: Record<string, unknown> | undefined) => {

‎packages/plugins/plugin-dev/src/dev-i18n.ts‎

Lines changed: 25 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,26 @@ const declaresTranslationArray = (body: unknown): boolean => {
9595
* read the top-level `translations` a second time — the same answer, reached
9696
* twice, for every single-package stack the platform has ever emitted.
9797
*
98+
* ## Why this reader KEEPS a private guard, and what the guard may not decide
99+
*
100+
* The rule for which `packages` values are absent and which are refused is
101+
* stated once, beside `AssembledPackageBodySchema` (`@objectstack/spec`,
102+
* `stack.zod.ts`). `resolveArtifactPackageOrder` is where it is enforced. A
103+
* reader should not spell it a second time, and the security reader's twin of
104+
* this guard was dropped for that reason. This one stays, deliberately, because
105+
* dropping it was measured NOT to be behaviour-equal here. Its answer and its
106+
* refusals do not move, but a single-package stack then reads its
107+
* `translations` twice instead of once. The pin "a single-package stack reads
108+
* its `translations` ONCE" (`dev-i18n-packages-reader.test.ts`) holds this
109+
* reader to exactly the old path, and it goes red without the guard.
110+
*
111+
* ⛔ So the guard is allowed to decide ONE thing: the ABSENT branch, spelled
112+
* exactly as the resolver's own absent branch (`undefined` / `null`). Every
113+
* other value goes to the resolver, so a present non-array `packages` (`{}`,
114+
* `0`, `'x'`) is REFUSED as `INVALID_ARTIFACT_PACKAGES`. ⛔ Never widen it to
115+
* `Array.isArray`: that is the silent fall-through the rule above forbids. If
116+
* the resolver's absent branch ever changes, this line changes with it.
117+
*
98118
* ## A malformed `packages[]` is refused, not skipped
99119
*
100120
* A non-array `packages`, an entry inlined instead of wrapped under `manifest:`
@@ -120,17 +140,11 @@ const declaresTranslationArray = (body: unknown): boolean => {
120140
* maintainer question filed separately; it is not decided here, and this
121141
* function's own semantics are unchanged by it.
122142
*
123-
* ## The guard divergence with `@objectstack/core`, recorded rather than fixed
124-
*
125-
* This reader treats only an ABSENT `packages` key (`undefined` / `null`) as
126-
* "single package"; anything else goes to the gate, so `packages: {}` is
127-
* REFUSED. `resolveArtifactPackageOrder`'s own second branch is spelled
128-
* `declared === undefined || declared === null` too, but the sibling reader in
129-
* `@objectstack/metadata` guards with `Array.isArray`, which silently accepts a
130-
* non-array. Two readers in one program answering the same input differently is
131-
* a program-level split, not this file's to settle (#15226 spells it as this
132-
* file does). Recorded here so the next author does not "fix" one side into
133-
* agreement without ruling the other.
143+
* ⚠️ The refusal is reached only when the question reaches the package pass. A
144+
* stack whose top-level `translations` already answers `true` returns before
145+
* the walk, so its `packages` is never read here, malformed or not. That is the
146+
* top-level-first order above, and it is deliberate: this reader refuses what
147+
* it READS. Such a stack is refused by the load path instead.
134148
*
135149
* ## `i18n` is NOT read from `packages[]`, and that is not an omission
136150
*

‎packages/plugins/plugin-security/src/app-default-permission-set.test.ts‎

Lines changed: 26 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -339,12 +339,36 @@ describe('appSecurityPluginOptions over `packages[]` (ADR-0130 D4, #15007)', ()
339339
}
340340
};
341341

342-
it('`packages` that is not an array', () => {
343-
const err = refusalOf({ packages: 'nope' });
342+
// A `packages` that is present but is not an array is MALFORMED, not absent
343+
// (the rule beside `AssembledPackageBodySchema`). This reader keeps no
344+
// `packages` guard of its own, so the refusal is the resolver's.
345+
it.each([
346+
['{}', {}],
347+
['0', 0],
348+
["'x'", 'x'],
349+
])('`packages: %s`, which is not an array', (_label, packages) => {
350+
const err = refusalOf({ packages });
344351
expect(err.code).toBe('INVALID_ARTIFACT_PACKAGES');
345352
expect(err.status).toBe(422);
346353
});
347354

355+
it('lit controls for the rows above: a well-formed `packages[]` resolves, and an absent key takes the single-package branch', () => {
356+
// Without these, an instrument that always threw would pin the three
357+
// rows above just as green.
358+
const wellFormed = { manifest: { id: CORE_ID, name: 'Core', version: '1.0.0', type: 'app', permissions: [permissionSet(CORE_PROFILE)] } };
359+
expect(refusalOf({ packages: [wellFormed] })).toEqual({});
360+
expect(appSecurityPluginOptions({ packages: [wellFormed] })).toEqual({ fallbackPermissionSet: CORE_PROFILE });
361+
362+
// Absent, explicitly `undefined`, and `null`: all three read the top level
363+
// exactly as before the private guard was dropped.
364+
for (const absent of [{}, { packages: undefined }, { packages: null }]) {
365+
expect(refusalOf({ ...absent, permissions: [permissionSet('top')] })).toEqual({});
366+
expect(appSecurityPluginOptions({ ...absent, permissions: [permissionSet('top')] }))
367+
.toEqual({ fallbackPermissionSet: 'top' });
368+
expect(appSecurityPluginOptions({ ...absent })).toBeUndefined();
369+
}
370+
});
371+
348372
it('an entry inlined instead of wrapped under `manifest:`', () => {
349373
const err = refusalOf({ packages: [{ id: CORE_ID, name: 'Core', version: '1.0.0', type: 'app', permissions: [permissionSet(CORE_PROFILE)] }] });
350374
expect(err.code).toBe('INVALID_ARTIFACT_PACKAGE_ENTRY');

‎packages/plugins/plugin-security/src/app-default-permission-set.ts‎

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -169,18 +169,25 @@ export function appDefaultPermissionSetName(permissions: unknown): string | unde
169169
* loadable or refused, and which answer this reader gives about it must not
170170
* depend on whether its flattened level happened to name a default first.
171171
*
172+
* ⛔ This reader has no `packages` guard of its own. Which `packages` values
173+
* are absent and which are refused is answered by `resolveArtifactPackageOrder`
174+
* alone. The rule is stated once, beside `AssembledPackageBodySchema`
175+
* (`@objectstack/spec`, `stack.zod.ts`). A private `undefined` / `null` check
176+
* here would be a second spelling of that answer.
177+
*
172178
* ## One thing it deliberately does NOT do
173179
*
174180
* It does not look inside the SINGULAR `manifest`. That constraint is #7001's
175181
* and it still holds — the harness must not honour a declaration `serve.ts`
176-
* ignores. Note this is not a special case bolted on: an artifact carrying no
177-
* `packages` key never reaches the package pass at all, so that branch reads
178-
* `permissions` from exactly where the old code read it and nowhere else.
182+
* ignores. Note this is not a special case bolted on. When the artifact carries
183+
* no `packages` key, D4's second branch hands back the artifact ITSELF as the
184+
* one package body. So the package pass re-reads `permissions` from exactly
185+
* where the flattened read did, the top level, and nowhere else. That re-read
186+
* cannot change the answer, because the flattened read already returned
187+
* `undefined` for the same value.
179188
*/
180189
function declaredDefaultPermissionSetName(config: unknown): string | undefined {
181-
const packages = (config as { packages?: unknown } | null | undefined)?.packages;
182-
const bodies =
183-
packages === undefined || packages === null ? [] : resolveArtifactPackageOrder(config);
190+
const bodies = resolveArtifactPackageOrder(config);
184191

185192
const flattened = (config as { permissions?: unknown } | null | undefined)?.permissions;
186193
const fromFlattened = appDefaultPermissionSetName(flattened);

‎packages/runtime/src/artifact-collections.test.ts‎

Lines changed: 40 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,7 @@ describe('packageOwnedCollectionKeys', () => {
8080
});
8181

8282
describe('resolveArtifactCollections', () => {
83-
it('returns the ARGUMENT ITSELF for anything without `packages[]`', () => {
83+
it('returns the ARGUMENT ITSELF for a non-object and for an ABSENT `packages`', () => {
8484
// The D7 branch: every single-package artifact and every `defineStack()`
8585
// config the platform has ever booted takes it, and identity is the only
8686
// way to say "this cannot have moved" rather than to hope so.
@@ -89,10 +89,45 @@ describe('resolveArtifactCollections', () => {
8989
expect(resolveArtifactCollections(null)).toBe(null);
9090
expect(resolveArtifactCollections(undefined)).toBe(undefined);
9191
expect(resolveArtifactCollections('not an object')).toBe('not an object');
92-
// `packages` present but not an array is not a shape this walks; the
93-
// artifact's own loader refuses it.
94-
const odd = { packages: 'nope', objects: [obj('o')] };
95-
expect(resolveArtifactCollections(odd)).toBe(odd);
92+
// An explicit `undefined`, and `null`, read as absent too. `null` is read
93+
// this way by every reader today; the schema's `.optional()` refuses it,
94+
// and that disagreement is recorded beside `AssembledPackageBodySchema`
95+
// rather than decided here.
96+
const explicitUndefined = { packages: undefined, objects: [obj('o')] };
97+
expect(resolveArtifactCollections(explicitUndefined)).toBe(explicitUndefined);
98+
const nullPackages = { packages: null, objects: [obj('o')] };
99+
expect(resolveArtifactCollections(nullPackages)).toBe(nullPackages);
100+
});
101+
102+
// A `packages` that is present but is not an array is MALFORMED, not absent
103+
// (the rule beside `AssembledPackageBodySchema`). This reader used to hand
104+
// such an artifact back by identity, answering about its top level while
105+
// the loader refused the same bytes.
106+
it.each([
107+
['{}', {}],
108+
['0', 0],
109+
["'x'", 'x'],
110+
])('refuses `packages: %s` with the resolver\'s INVALID_ARTIFACT_PACKAGES envelope', (_label, packages) => {
111+
let raised: any;
112+
try {
113+
resolveArtifactCollections({ manifest: { id: 'com.example.a', name: 'A' }, objects: [obj('o')], packages });
114+
} catch (err) {
115+
raised = err;
116+
}
117+
expect(raised?.code).toBe('INVALID_ARTIFACT_PACKAGES');
118+
expect(raised?.status).toBe(422);
119+
});
120+
121+
it('lit controls for the refusal above: a well-formed `packages[]` resolves, and an absent key takes the single-package branch', () => {
122+
// Without these two, an instrument that always threw would pin the
123+
// three rows above just as green.
124+
const wellFormed = resolveArtifactCollections({
125+
packages: packagesOf({ objects: [obj('account')] }, { objects: [obj('order')] }),
126+
}) as Record<string, any>;
127+
expect(wellFormed.objects.map((o: any) => o.name)).toEqual(['account', 'order']);
128+
129+
const absent = { manifest: { id: 'com.example.a', name: 'A' }, objects: [obj('o')] };
130+
expect(resolveArtifactCollections(absent)).toBe(absent);
96131
});
97132

98133
it('returns the ARGUMENT ITSELF for an EMPTY `packages: []` too', () => {

0 commit comments

Comments
 (0)