fix(cli): refuse a present non-array packages in the stack-collection and docs readers

.changeset/19925-cli-non-array-packages-refusal.md

@@ -0,0 +1,46 @@
+---
+'@objectstack/cli': minor
+---
+
+fix(cli): `os info` and `os lint` refuse a stack whose `packages` is present but not an array, instead of reading it as no packages (#19925)
+
+Clause-②: no (narrowing)
+
+**BREAKING for `os info` and `os lint` on a hand-written stack.** A config whose
+`packages` is present but is not an array (`{}`, `0`, `'x'`) is now refused with
+`INVALID_ARTIFACT_PACKAGES` (ADR-0112, `status: 422`) and exit code 1. These
+commands used to read it as a stack with no packages. `os info` exited 0 and
+reported every package-owned collection as empty. `os lint` exited 0 with
+`passed: true`. Only two spellings reach these commands with such a value: a
+config exported as a plain object, and `defineStack(…, { strict: false })`.
+The default `defineStack` parse, `os validate` and `os build` already refused
+it.
+
+The accept set only shrinks back to what the declaration has always said.
+`ObjectStackDefinitionSchema` declares `packages` as an array of package
+entries. Ruling A on #15293 settled that a present non-array value is
+malformed, not absent. The runtime, `@objectstack/core` and the plugin readers
+already refused it. The CLI's stack-collection reader and its three
+package-docs readers still answered "no packages". They now judge the value
+through one helper, which hands a non-array to `resolveArtifactPackageOrder`.
+So the refusal's code, status and sentence are core's own, and the CLI keeps no
+second copy of the rule.
+
+**What is not affected.** An absent `packages` reads exactly as before, and so
+does a `packages` array. A malformed entry inside an array is still refused as
+`INVALID_ARTIFACT_PACKAGE_ENTRY`. `os serve` and `os dev` refused this stack
+before the change, because the runtime's manifest service raises the same
+refusal at boot, and they still do.
+
+**`packages: null` follows core's resolver.** Ruling `5805260775` on #19926
+makes `null` malformed at every reader, and the resolver's `null` refusal is
+landing separately (#19926, PR #20228). The CLI readers do not answer `null`
+themselves; they hand it to `resolveArtifactPackageOrder` and return what it
+answers. Today that is its absent answer, so `null` still reads as no
+packages. Once the resolver refuses `null`, these commands refuse it too, with
+no change to the CLI.
+
+**If you are refused.** Omit `packages` for a single-package stack, or give it
+an array of `{ manifest: … }` entries. The refusal says the same.
+
+<!-- adr-0087: not-required (no-migration-prescription) Nothing authorable is removed, renamed or reshaped: no spec key, no export, no stored row. `objectstack migrate meta` has nothing to reach, because a non-array `packages` was never a legal spelling of anything, so no old form maps to a new one. The refusal itself carries the remedy. -->

packages/cli/src/utils/collect-docs.package-docs.test.ts

@@ -746,9 +746,20 @@ describe('docsPackageRefs', () => {
     ]);
   });
 
-  it('is empty for anything that is not an array', () => {
+  it('is empty for an absent `packages`, and refuses a present non-array one', () => {
     expect(docsPackageRefs(undefined)).toEqual([]);
-    expect(docsPackageRefs({})).toEqual([]);
+    // #19925: `{}` is malformed, not absent (ruling A on #15293). This row used
+    // to pin `[]` for it, which was the silent answer that card removes. The
+    // full `{}` / `0` / `'x'` set is pinned in
+    // `test/non-array-packages-readers.test.ts`.
+    let refusal: { code?: unknown; status?: unknown } | undefined;
+    try {
+      docsPackageRefs({});
+    } catch (error) {
+      refusal = error as { code?: unknown; status?: unknown };
+    }
+    expect(refusal?.code).toBe('INVALID_ARTIFACT_PACKAGES');
+    expect(refusal?.status).toBe(422);
   });
 });
 

packages/cli/src/utils/collect-docs.ts

@@ -64,6 +64,7 @@ import fs from 'fs';
 import path from 'path';
 
 import { artifactPackages } from './artifact-packages.js';
+import { declaredPackageEntries } from './stack-collections.js';
 
 export interface DocTranslationItem {
   label?: string;
@@ -380,10 +381,14 @@ export interface PackageDocSet {
  *
  * A directory that matches none, or more than one, is not attributed — it is
  * reported, by {@link sweepPackageDocsDirectories}.
+ *
+ * An absent `packages` answers `[]`. A present non-array one (`{}`, `0`, `'x'`)
+ * is refused with the resolver's `INVALID_ARTIFACT_PACKAGES` through
+ * {@link declaredPackageEntries}; it is never read as "no packages" (#19925).
+ * `null` gets whatever the resolver answers for it; see that function.
  */
 export function docsPackageRefs(packages: unknown): DocsPackageRef[] {
-  if (!Array.isArray(packages)) return [];
-  return artifactPackages({ packages }).map(({ index, id, body }) => {
+  return artifactPackages({ packages: declaredPackageEntries(packages) }).map(({ index, id, body }) => {
     const directoryNames = new Set<string>();
     if (typeof body.id === 'string' && body.id !== '') {
       directoryNames.add(body.id);
@@ -1158,10 +1163,16 @@ function claimedDocs(items: readonly DocItem[]): { has: (doc: DocItem) => boolea
   };
 }
 
-/** The `docs` a `packages[]` entry already carries in its own assembled body. */
+/**
+ * The `docs` a `packages[]` entry already carries in its own assembled body.
+ *
+ * `packages` is judged by {@link declaredPackageEntries}, so a present
+ * non-array value goes to the resolver rather than being read as an entry with
+ * no docs.
+ */
 function bodyDocsOf(packages: unknown, index: number): DocItem[] {
-  if (!Array.isArray(packages)) return [];
-  const body = (packages[index] as { manifest?: Record<string, unknown> } | null | undefined)?.manifest;
+  const entry = declaredPackageEntries(packages)[index];
+  const body = (entry as { manifest?: Record<string, unknown> } | null | undefined)?.manifest;
   const docs = body?.docs;
   return Array.isArray(docs) ? (docs as DocItem[]) : [];
 }
@@ -1358,12 +1369,20 @@ export function collectAndLintDocs(
  * Returns the ARGUMENT ITSELF when nothing is added, so an artifact with no
  * per-package docs is not merely equal to the one built before this landed —
  * it is the same object, serialized from the same references.
+ *
+ * `packages` is judged FIRST, by {@link declaredPackageEntries}. An absent one
+ * comes back as it came in. A present non-array one (`{}`, `0`, `'x'`) is
+ * refused, because handing it back unchanged would carry it into the artifact
+ * as if it held no packages (#19925). `null` gets whatever the resolver
+ * answers for it: today that is the absent answer, so `null` comes back as it
+ * came in.
  */
 export function attachPackageDocs(packages: unknown, sets: readonly PackageDocSet[]): unknown {
-  if (!Array.isArray(packages) || sets.length === 0) return packages;
+  const entries = declaredPackageEntries(packages);
+  if (entries.length === 0 || sets.length === 0) return packages;
   const byIndex = new Map(sets.map((set) => [set.index, set]));
   let changed = false;
-  const out = packages.map((entry, index) => {
+  const out = entries.map((entry, index) => {
     const set = byIndex.get(index);
     if (!set || set.docs.length === 0) return entry;
     const body = (entry as { manifest?: Record<string, unknown> } | null | undefined)?.manifest;

packages/cli/src/utils/stack-collections.ts

@@ -84,6 +84,78 @@ type Bag = Record<string, unknown>;
 const asBag = (value: unknown): Bag | undefined =>
   value && typeof value === 'object' ? (value as Bag) : undefined;
 
+/**
+ * A stack's `packages` value, judged ONCE for every reader in this package that
+ * walks it: the entries, by position, or `[]` when the key is absent.
+ *
+ * ## A present non-array `packages` is refused, never read as "no packages"
+ *
+ * A `packages` that is present but is not an array (`{}`, `0`, `'x'`) is
+ * MALFORMED, not absent (ruling A on #15293). The rule is stated once, beside
+ * `AssembledPackageBodySchema` (`@objectstack/spec`, `stack.zod.ts`), and it is
+ * enforced by `resolveArtifactPackageOrder` (`@objectstack/core`), which
+ * refuses the value as `INVALID_ARTIFACT_PACKAGES` (ADR-0112, `status: 422`).
+ * The runtime, `@objectstack/core` and the plugin readers already refuse it.
+ * This package's readers used to answer "no packages" instead (#19925): `os
+ * info` printed `0` objects for such a stack and `os lint` passed it. So this
+ * function spells neither the rule nor the refusal. It hands a non-array value
+ * to the resolver, and the refusal the author sees is the resolver's own.
+ *
+ * - The key ABSENT (`undefined`) answers `[]`. This is the only value the
+ *   function answers on its own.
+ * - An array is returned BY REFERENCE and unparsed. The docs readers need
+ *   entries by POSITION (`packages[i]` is where collected docs attach), and the
+ *   resolver answers bodies in LOAD order, so they cannot read its result.
+ *   Parsing each entry is the resolver's job on the path that registers
+ *   packages ({@link packageBodies} reaches it). Adding that parse to the docs
+ *   readers would widen what they refuse, and that is a separate change.
+ * - Every other value goes to the resolver, `null` included. A non-array is
+ *   refused there.
+ *
+ * ## `null` follows the resolver, and is never judged here
+ *
+ * Ruling A on #19926 (`5805260775`) settles `null`: it is malformed at every
+ * reader, and `resolveArtifactPackageOrder` drops its `null` branch. That core
+ * change lands separately (#19926), and until it does, the resolver still
+ * answers `null` through its ABSENT branch. So this function does not answer
+ * `null` itself. It asks the resolver and reads the answer:
+ *
+ * - The resolver's absent answer is `[artifact]`, holding the caller's own
+ *   object BY REFERENCE (ADR-0130 D4, second branch). This function recognises
+ *   that answer by IDENTITY against the object it passed in, and returns `[]`.
+ *   That is today's answer for `null`, byte for byte.
+ * - Once the resolver refuses `null`, its `INVALID_ARTIFACT_PACKAGES` reaches
+ *   every reader here, with no edit to this package.
+ *
+ * ⛔ Never add a private `null` branch here, in either direction. Answering
+ * `null` as absent here would keep the CLI reading it as absent after the
+ * resolver starts refusing it. Refusing it here would be a second copy of a
+ * rule the resolver owns.
+ *
+ * ⛔ Never put an `Array.isArray` in front of this function as a fall-through
+ * to "no packages". That silent answer is exactly what this function removes.
+ *
+ * @throws Whatever the resolver raises for a present non-array `packages`:
+ *   today `INVALID_ARTIFACT_PACKAGES` for every non-array except `null`.
+ */
+export function declaredPackageEntries(packages: unknown): readonly unknown[] {
+  // The key is absent: there is nothing to judge.
+  if (packages === undefined) return [];
+  if (Array.isArray(packages)) return packages;
+  // Present and not an array, `null` included: the resolver decides.
+  const probe = { packages };
+  const answer = resolveArtifactPackageOrder(probe);
+  // The resolver's ABSENT answer holds the probe itself, by reference.
+  if (answer.length === 1 && answer[0] === probe) return [];
+  // Reached only if the resolver answers a non-array with anything but a
+  // refusal or its absent answer. An empty answer here would bring back the
+  // silent fall-through, so fail loudly.
+  throw new Error(
+    `resolveArtifactPackageOrder accepted a \`packages\` of type ${packages === null ? 'null' : typeof packages}; `
+    + 'the CLI package readers cannot walk it by position.',
+  );
+}
+
 /**
  * The assembled package bodies this stack carries, in dependency-topological
  * order — or `[]` when it carries no `packages` list of its own.
@@ -94,10 +166,13 @@ const asBag = (value: unknown): Bag | undefined =>
  * a stack's own top level back onto itself resolves nothing — so this returns
  * an empty list for that case, and every caller below reads the top level
  * first anyway.
+ *
+ * A `packages` that is present but is not an array goes through
+ * {@link declaredPackageEntries}, which hands it to the resolver: it is refused,
+ * or, for `null` while the resolver still reads it as absent, answered `[]`.
  */
 function packageBodies(stack: unknown): Bag[] {
-  const declared = asBag(stack)?.packages;
-  if (!Array.isArray(declared) || declared.length === 0) return [];
+  if (declaredPackageEntries(asBag(stack)?.packages).length === 0) return [];
   return (resolveArtifactPackageOrder(stack) as unknown[])
     .map(asBag)
     .filter((b): b is Bag => b !== undefined);

packages/cli/test/non-array-packages-readers.test.ts

@@ -0,0 +1,166 @@
+// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
+
+/**
+ * A `packages` that is present but is not an array is REFUSED by every
+ * `@objectstack/cli` reader of it, never read as "no packages" (#19925).
+ *
+ * The rule is ruling A on #15293: `{}`, `0` and `'x'` are malformed, not
+ * absent. It is stated once, beside `AssembledPackageBodySchema`
+ * (`@objectstack/spec`), and enforced once, by `resolveArtifactPackageOrder`
+ * (`@objectstack/core`), as `INVALID_ARTIFACT_PACKAGES`. The runtime, core and
+ * the plugin readers already refused it. The CLI readers answered `[]` or
+ * handed the value back unchanged, so `os info` printed `0` objects for such a
+ * stack and `os lint` passed it with exit 0. Both were measured from a
+ * hand-written config exported as a plain object, which reaches the command
+ * without `defineStack`'s parse.
+ *
+ * Each reader is pinned through the entry its door calls:
+ *
+ * - `packageBodies` (private) through `resolveStackCollection` and
+ *   `authoringRuleUnionStack`, and through the two door functions that reach
+ *   it first: `collectMetadataStats` (`os info`) and `lintConfig` (`os lint`).
+ * - `docsPackageRefs`, directly and through `collectDocsFromSrc`, the
+ *   collection step `os serve` / `os build` / `os lint` run.
+ * - `attachPackageDocs`, directly.
+ * - `bodyDocsOf` (private) has one caller, and that caller has already handed
+ *   the same value to `docsPackageRefs`, so a non-array cannot reach it
+ *   through any export. It goes through the same judgment and is not pinned
+ *   separately.
+ *
+ * Every refusal asserts the ADR-0112 envelope (`code` + `status`), never a bare
+ * `toThrow()`, which an unnamed `Error` would also satisfy. The lit control is a
+ * malformed ENTRY, refused as `INVALID_ARTIFACT_PACKAGE_ENTRY` through the same
+ * entries. The other controls are a well-formed array, which reads as before,
+ * and an absent `packages`, which still reads as no packages.
+ *
+ * `packages: null` is pinned in `null-packages-follows-resolver.test.ts`, not
+ * here. Ruling A on #19926 (`5805260775`) makes `null` malformed at every
+ * reader, and it is refused by core's resolver, never by the CLI. So the pin
+ * there asserts that each reader answers `null` the way the resolver does. It
+ * does not assert a fixed answer.
+ */
+
+import { describe, expect, it } from 'vitest';
+
+import { lintConfig } from '../src/commands/lint';
+import { attachPackageDocs, collectDocsFromSrc, docsPackageRefs, type PackageDocSet } from '../src/utils/collect-docs';
+import { collectMetadataStats } from '../src/utils/format';
+import { authoringRuleUnionStack, resolveStackCollection } from '../src/utils/stack-collections';
+
+const MANIFEST = {
+  id: 'com.example.probe',
+  name: 'probe',
+  version: '1.0.0',
+  type: 'app' as const,
+  namespace: 'probe',
+  engines: { protocol: '^17' },
+};
+
+const PROBE_OBJECT = {
+  name: 'probe_account',
+  label: 'Probe Account',
+  sharingModel: 'private' as const,
+  fields: { name: { name: 'name', type: 'text' as const, label: 'Name' } },
+};
+
+/** The three spellings ruling A names. */
+const NON_ARRAYS: ReadonlyArray<readonly [string, unknown]> = [
+  ['{}', {}],
+  ['0', 0],
+  ["'x'", 'x'],
+];
+
+/** One well-formed entry, as `resolveArtifactPackageOrder` parses it whole. */
+const wellFormed = () => [{ manifest: { ...MANIFEST, objects: [PROBE_OBJECT] } }];
+
+/** The lit control: a body inlined onto the array element, not wrapped. */
+const inlinedEntry = () => [{ ...MANIFEST, objects: [PROBE_OBJECT] }];
+
+/** A stack whose ONLY answer to "which objects" has to come from `packages`. */
+const stackWith = (packages: unknown) => ({ manifest: MANIFEST, packages });
+
+const docSet = (): PackageDocSet[] => [{
+  index: 0,
+  id: MANIFEST.id,
+  namespace: MANIFEST.namespace,
+  dir: 'src/probe/docs',
+  docs: [{ name: 'probe_guide', label: 'Guide', content: '# Guide' }],
+}];
+
+/** A config path whose `src/` does not exist: nothing on disk is read. */
+const CONFIG_PATH = '/nonexistent-19925/objectstack.config.ts';
+
+/** The ADR-0112 envelope a call raised, or `undefined` when it returned. */
+function refusalOf(call: () => unknown): { code?: unknown; status?: unknown } | undefined {
+  try {
+    call();
+  } catch (error) {
+    expect(error, 'the refusal is an Error carrying the envelope').toBeInstanceOf(Error);
+    return error as { code?: unknown; status?: unknown };
+  }
+  return undefined;
+}
+
+function expectRefused(call: () => unknown, code: string): void {
+  const refusal = refusalOf(call);
+  expect(refusal, `expected a ${code} refusal, got an answer`).toBeDefined();
+  expect(refusal?.code).toBe(code);
+  expect(refusal?.status).toBe(422);
+}
+
+describe('#19925: a present non-array `packages` is refused by every CLI reader', () => {
+  describe.each(NON_ARRAYS)('packages: %s', (_label, packages) => {
+    it('packageBodies refuses through resolveStackCollection and authoringRuleUnionStack', () => {
+      expectRefused(() => resolveStackCollection(stackWith(packages), 'objects'), 'INVALID_ARTIFACT_PACKAGES');
+      expectRefused(() => authoringRuleUnionStack(stackWith(packages)), 'INVALID_ARTIFACT_PACKAGES');
+    });
+
+    it('`os info` (collectMetadataStats) and `os lint` (lintConfig) refuse instead of answering', () => {
+      expectRefused(() => collectMetadataStats(stackWith(packages)), 'INVALID_ARTIFACT_PACKAGES');
+      expectRefused(() => lintConfig(stackWith(packages)), 'INVALID_ARTIFACT_PACKAGES');
+    });
+
+    it('docsPackageRefs refuses, directly and through collectDocsFromSrc', () => {
+      expectRefused(() => docsPackageRefs(packages), 'INVALID_ARTIFACT_PACKAGES');
+      expectRefused(() => collectDocsFromSrc(CONFIG_PATH, packages), 'INVALID_ARTIFACT_PACKAGES');
+    });
+
+    it('attachPackageDocs refuses rather than handing the value back', () => {
+      expectRefused(() => attachPackageDocs(packages, docSet()), 'INVALID_ARTIFACT_PACKAGES');
+    });
+  });
+});
+
+describe('#19925 controls: the same entries still read the well-formed and absent shapes', () => {
+  it('a well-formed array reads as before', () => {
+    expect(resolveStackCollection(stackWith(wellFormed()), 'objects')).toEqual([PROBE_OBJECT]);
+    expect(collectMetadataStats(stackWith(wellFormed())).objects).toBe(1);
+    expect(docsPackageRefs(wellFormed()).map((ref) => ref.id)).toEqual([MANIFEST.id]);
+    expect(collectDocsFromSrc(CONFIG_PATH, wellFormed()).packageDocs).toEqual([]);
+
+    const attached = attachPackageDocs(wellFormed(), docSet()) as Array<{ manifest: { docs?: unknown[] } }>;
+    expect(attached[0].manifest.docs).toEqual(docSet()[0].docs);
+  });
+
+  it('an empty array reads as no packages, and attachPackageDocs hands it back by identity', () => {
+    const empty: unknown[] = [];
+    expect(resolveStackCollection(stackWith(empty), 'objects')).toEqual([]);
+    expect(docsPackageRefs(empty)).toEqual([]);
+    expect(attachPackageDocs(empty, docSet())).toBe(empty);
+  });
+
+  it('an absent `packages` still reads as no packages', () => {
+    const absent = { manifest: MANIFEST };
+    expect(resolveStackCollection(absent, 'objects')).toEqual([]);
+    expect(authoringRuleUnionStack(absent)).toBe(absent);
+    expect(collectMetadataStats(absent).objects).toBe(0);
+    expect(docsPackageRefs(undefined)).toEqual([]);
+    expect(attachPackageDocs(undefined, docSet())).toBeUndefined();
+  });
+
+  it('lit control: a malformed ENTRY is refused through the same entries, with its own code', () => {
+    expectRefused(() => resolveStackCollection(stackWith(inlinedEntry()), 'objects'), 'INVALID_ARTIFACT_PACKAGE_ENTRY');
+    expectRefused(() => collectMetadataStats(stackWith(inlinedEntry())), 'INVALID_ARTIFACT_PACKAGE_ENTRY');
+    expectRefused(() => lintConfig(stackWith(inlinedEntry())), 'INVALID_ARTIFACT_PACKAGE_ENTRY');
+  });
+});