From 6a555d244cf8f051d30b95ed4a5a69829a26f091 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 4 Sep 2026 05:53:10 +0000 Subject: [PATCH 1/7] feat(runtime): every top-level collection read gains a `packages[]` path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ADR-0130 D4 / option B, reader program 2/4 (#15005). Nothing here changes what any command emits — the artifact stays additive; each reader simply learns to find its collections under `packages[]` as well as at the top level. `resolveArtifactCollections` (`@objectstack/core`, beside `resolveArtifactPackageOrder`) is the one resolution: top level first and whole, then every package body's items the top level did not already claim, in `resolveArtifactPackageOrder`'s order. On a bundle without `packages[]` it returns the argument itself, so every single-package artifact and every `defineStack()` config is bit-identical. Readers taught: `AppPlugin` (datasources, datasourceMapping, objects, jobs, seed data, translations, the ADR-0057 security block, the job handler context's bundle), the three exported collectors, `mergeRuntimeModule`'s declaration half, `createStandaloneStack`'s surfaced keys, and `resolve-project-database`'s project-DB tier. #15004's acceptance pin shrinks from 24 ledgered losses to 1 — the from-source `appSecurityPluginOptions` row card #15007 owns. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01UHvF5hyiZjnCyExFnfQB8m --- .../option-b-reader-acceptance.pin.test.ts | 40 +-- .../core/src/artifact-collections.test.ts | 225 ++++++++++++++ packages/core/src/artifact-collections.ts | 284 ++++++++++++++++++ packages/core/src/index.ts | 8 + .../src/app-plugin.option-b-packages.test.ts | 211 +++++++++++++ packages/runtime/src/app-plugin.ts | 121 ++++++-- packages/runtime/src/load-artifact-bundle.ts | 23 +- .../runtime/src/resolve-project-database.ts | 18 +- packages/runtime/src/standalone-stack.ts | 25 +- 9 files changed, 890 insertions(+), 65 deletions(-) create mode 100644 packages/core/src/artifact-collections.test.ts create mode 100644 packages/core/src/artifact-collections.ts create mode 100644 packages/runtime/src/app-plugin.option-b-packages.test.ts diff --git a/packages/cli/test/option-b-reader-acceptance.pin.test.ts b/packages/cli/test/option-b-reader-acceptance.pin.test.ts index 62ae5777d1..ea3be138f6 100644 --- a/packages/cli/test/option-b-reader-acceptance.pin.test.ts +++ b/packages/cli/test/option-b-reader-acceptance.pin.test.ts @@ -118,36 +118,28 @@ import { measureShape, type ProbeRow, type ShapeMeasurement } from './fixtures/o /** * The subsystems that silently lose their collection when the flattened top - * level is gone — MEASURED on `origin/main` `33681eaef`, not curated. + * level is gone — MEASURED, not curated. Opened at 24 rows on `origin/main` + * `33681eaef` (#15004); 23 of them were deleted by #15005 when + * `@objectstack/runtime` learned to resolve `packages[]` — the whole of + * boundaries B1 and B5, plus every B2 row whose reader ships in that package. + * + * The one row left is NOT a runtime reader: `appSecurityPluginOptions` + * (`@objectstack/plugin-security`) reads `config.permissions` off the + * from-source config directly, so nothing on the artifact side can reach it. + * Card #15007 owns it, and the ledger is empty — the reader half done, the + * emitter half #14512 unblocked — when it goes. + * + * Its B1 twin is already gone: that row runs `appSecurityPluginOptions` over + * `createStandaloneStack`'s RESULT, and the result now surfaces `permissions` + * resolved across both shapes. Same reader, two boundaries, one of them fed by + * a producer this card fixed — which is the two-boundary split #15005 warns + * about, seen from the ledger's side. * * ⛔ SHRINK-ONLY, audited in BOTH directions (see the header). Each line names * a boundary, a subsystem and the collection it reads. */ const OPTION_B_LOSSES: readonly string[] = [ - 'B1 · AppPlugin declared-datasource auto-connect (compiled artifact) · datasources', - 'B1 · AppPlugin job scheduling (compiled artifact) · jobs', - 'B1 · AppPlugin objects handed to datasource connect (compiled artifact) · objects', - 'B1 · AppPlugin ql.setDatasourceMapping (object routing) (compiled artifact) · datasourceMapping', - 'B1 · AppPlugin seed datasets merged (compiled artifact) · data', - 'B1 · AppPlugin translation loading into the i18n service (compiled artifact) · translations', - 'B1 · createStandaloneStack surfaced objects (CLI tier resolution + engine/driver auto-registration) · objects', - 'B1 · createStandaloneStack surfaced permissions (ADR-0056 D7) · permissions', - 'B1 · createStandaloneStack surfaced positions · positions', - 'B1 · plugin-security appSecurityPluginOptions over the artifact-serve config (default permission set) · permissions', - 'B1 · runtime collectBundleActions (action dispatch registration) · actions + objects[].actions', - 'B1 · runtime collectBundleFunctionEntries (declared function effect) · functions', - 'B1 · runtime collectBundleHooks (declarative hook binding) · hooks', - 'B2 · AppPlugin declared-datasource auto-connect (from source) · datasources', - 'B2 · AppPlugin job scheduling (from source) · jobs', - 'B2 · AppPlugin objects handed to datasource connect (from source) · objects', - 'B2 · AppPlugin ql.setDatasourceMapping (object routing) (from source) · datasourceMapping', - 'B2 · AppPlugin seed datasets merged (from source) · data', - 'B2 · AppPlugin translation loading into the i18n service (from source) · translations', 'B2 · plugin-security appSecurityPluginOptions over the from-source config (default permission set) · permissions', - 'B2 · runtime collectBundleActions over the from-source config · actions + objects[].actions', - 'B2 · runtime collectBundleFunctionEntries over the from-source config · functions', - 'B2 · runtime collectBundleHooks over the from-source config · hooks', - 'B5 · resolve-project-database readConfigDeclaredDefault (project database tier) · datasourceMapping + datasources', ]; const render = (rows: ProbeRow[], only?: (r: ProbeRow) => boolean): string => diff --git a/packages/core/src/artifact-collections.test.ts b/packages/core/src/artifact-collections.test.ts new file mode 100644 index 0000000000..74e2b3be6b --- /dev/null +++ b/packages/core/src/artifact-collections.test.ts @@ -0,0 +1,225 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * `resolveArtifactCollections` — ADR-0130 D4 / option B (#15005). + * + * The acceptance pin for the reader program + * (`packages/cli/test/option-b-reader-acceptance.pin.test.ts`) measures whether + * the SUBSYSTEMS see their collections. These tests pin the resolution itself, + * on the four properties that pin cannot separate because a real boot exercises + * them together: + * + * 1. the additive shape the platform emits TODAY comes back UNCHANGED — + * identity, not merely equality — so the reader program cannot have moved + * it; + * 2. an option-B artifact yields the package bodies' collections, in + * `resolveArtifactPackageOrder`'s order; + * 3. a PARTIALLY flattened artifact resolves per item, which is the state a + * "top level, else `packages[]`" fallback answers wrongly; + * 4. a key NO source declares stays ABSENT rather than becoming `[]` — + * `createStandaloneStack` omits `objects` on that basis and consumers gate + * on the key's presence. + */ + +import { describe, it, expect } from 'vitest'; + +import { resolveArtifactCollections, packageOwnedCollectionKeys } from './artifact-collections'; + +/** A schema-valid object definition — `ArtifactPackageSchema` parses each body WHOLE. */ +const obj = (name: string, fields: Record = {}): Record => ({ + name, + label: name, + fields: { name: { name: 'name', type: 'text', label: 'Name' }, ...fields }, +}); + +/** Two packages, `orders` depending on `core`, so the order is not the array's. */ +const packagesOf = ( + coreCollections: Record = {}, + ordersCollections: Record = {}, +): unknown[] => [ + { + manifest: { + id: 'com.example.orders', + name: 'Orders', + version: '1.0.0', + type: 'module', + dependencies: { 'com.example.core': '^1.0.0' }, + ...ordersCollections, + }, + }, + { + manifest: { id: 'com.example.core', name: 'Core', version: '1.0.0', type: 'app', ...coreCollections }, + }, +]; + +describe('packageOwnedCollectionKeys', () => { + it('is derived from the schemas — the collections a package owns, never the envelope', () => { + const keys = packageOwnedCollectionKeys(); + // A positive first, so the exclusions below are a measurement rather + // than an empty set agreeing with everything. + expect(keys.length).toBeGreaterThan(30); + for (const collection of ['objects', 'actions', 'hooks', 'jobs', 'data', 'translations', + 'datasources', 'datasourceMapping', 'permissions', 'positions', 'functions', 'requires']) { + expect(keys, `${collection} is a package-owned collection`).toContain(collection); + } + // The seven envelope keys an option-B artifact still carries at its top + // level. `packages` most of all: an artifact carries packages, a package + // inside it does not (ADR-0130 D1). + for (const envelope of ['manifest', 'packages', 'api', 'server', 'i18n', 'runtimeModule', 'onEnable']) { + expect(keys, `${envelope} is an envelope key`).not.toContain(envelope); + } + }); +}); + +describe('resolveArtifactCollections', () => { + it('returns the ARGUMENT ITSELF for anything without `packages[]`', () => { + // The D7 branch: every single-package artifact and every `defineStack()` + // config the platform has ever booted takes it, and identity is the only + // way to say "this cannot have moved" rather than to hope so. + const single = { manifest: { id: 'a', name: 'A' }, objects: [obj('o')] }; + expect(resolveArtifactCollections(single)).toBe(single); + expect(resolveArtifactCollections(null)).toBe(null); + expect(resolveArtifactCollections(undefined)).toBe(undefined); + expect(resolveArtifactCollections('not an object')).toBe('not an object'); + // `packages` present but not an array is not a shape this walks; the + // artifact's own loader refuses it. + const odd = { packages: 'nope', objects: [obj('o')] }; + expect(resolveArtifactCollections(odd)).toBe(odd); + }); + + it('leaves TODAY\'s additive artifact untouched — same arrays, same order, same references', () => { + const coreObject = obj('account'); + const ordersObject = obj('order'); + const objects = [coreObject, ordersObject]; + const rules = [{ datasource: 'primary', default: true }]; + const additive = { + manifest: { id: 'com.example.core', name: 'Core' }, + objects, + datasourceMapping: rules, + packages: packagesOf({ objects: [coreObject], datasourceMapping: rules }, { objects: [ordersObject] }), + }; + const resolved = resolveArtifactCollections(additive) as typeof additive; + expect(resolved.objects).toBe(objects); + expect(resolved.datasourceMapping).toBe(rules); + expect(resolved).toBe(additive); + }); + + it('claims a top-level copy STRUCTURALLY, so a JSON round-trip does not double it', () => { + // The compiled path: `packages[]` and the top level carry equal values + // that are no longer the same objects. Reference de-duplication alone + // would register every collection twice on every multi-package artifact + // the platform ships today. + const additive = { + translations: [{ en: { objects: { account: { label: 'Account' } } } }], + requires: ['platform'], + packages: packagesOf({ + translations: [{ en: { objects: { account: { label: 'Account' } } } }], + requires: ['platform'], + }), + }; + const roundTripped = JSON.parse(JSON.stringify(additive)); + const resolved = resolveArtifactCollections(roundTripped) as typeof additive; + expect(resolved.translations).toHaveLength(1); + expect(resolved.requires).toEqual(['platform']); + }); + + it('claims by NAME too, so a merged top-level object is not joined by its unmerged halves', () => { + // `objects` is the one collection `composeStacks` MERGES rather than + // concatenates, so the top-level entry and the two package bodies that + // produced it do not serialize alike. Deduplicating structurally alone + // would hand the reader three `account` objects. + const merged = obj('account', { a: { name: 'a', type: 'text', label: 'A' }, b: { name: 'b', type: 'text', label: 'B' } }); + const additive = { + objects: [merged], + packages: packagesOf( + { objects: [obj('account', { a: { name: 'a', type: 'text', label: 'A' } })] }, + { objects: [obj('account', { b: { name: 'b', type: 'text', label: 'B' } })] }, + ), + }; + const resolved = resolveArtifactCollections(additive) as typeof additive; + expect(resolved.objects).toEqual([merged]); + }); + + it('reads an option-B artifact out of `packages[]`, in package order', () => { + const optionB = { + manifest: { id: 'com.example.core', name: 'Core' }, + packages: packagesOf( + { objects: [obj('account')], permissions: [{ name: 'default_profile', label: 'Default', isDefault: true, objects: {} }] }, + { objects: [obj('order')], actions: [{ name: 'ship', label: 'Ship', type: 'script', body: { language: 'js', source: 'return 1;' } }] }, + ), + }; + const resolved = resolveArtifactCollections(optionB) as Record; + // `core` first — `resolveArtifactPackageOrder` sorts topologically, and + // `orders` DEPENDS on it, so this is not the array's own order. ⛔ The + // order is that function's; nothing here re-derives it. + expect(resolved.objects.map((o: any) => o.name)).toEqual(['account', 'order']); + expect(resolved.actions.map((a: any) => a.name)).toEqual(['ship']); + expect(resolved.permissions).toEqual([{ name: 'default_profile', label: 'Default', isDefault: true, objects: {} }]); + // Envelope keys are the caller's own references, untouched. + expect(resolved.manifest).toBe(optionB.manifest); + expect(resolved.packages).toBe(optionB.packages); + }); + + it('keeps BOTH same-named package bodies when nothing merged them', () => { + // The other half of the name rule: on an option-B artifact a base and + // its extension are two entries of one name and no top level claimed + // either. Deduplicating by name here would drop the extension. + const optionB = { + packages: packagesOf( + { objects: [obj('account', { a: { name: 'a', type: 'text', label: 'A' } })] }, + { objects: [obj('account', { b: { name: 'b', type: 'text', label: 'B' } })] }, + ), + }; + const resolved = resolveArtifactCollections(optionB) as Record; + expect(resolved.objects).toEqual([ + obj('account', { a: { name: 'a', type: 'text', label: 'A' } }), + obj('account', { b: { name: 'b', type: 'text', label: 'B' } }), + ]); + }); + + it('resolves a PARTIALLY flattened artifact per item, not per artifact', () => { + // The transition state: one package's collections are flattened, the + // other's are not. "Use the top level when present, else `packages[]`" + // answers this one wrongly and silently. + const partial = { + objects: [obj('account')], + packages: packagesOf({ objects: [obj('account')] }, { objects: [obj('order')] }), + }; + const resolved = resolveArtifactCollections(partial) as Record; + expect(resolved.objects.map((o: any) => o.name)).toEqual(['account', 'order']); + }); + + it('merges the RECORD spelling of a collection, top level winning', () => { + // `functions` is a map, and `datasources` is legitimately either shape. + const artifact = { + functions: { fromTop: () => 'top' }, + packages: packagesOf({ functions: { fromCore: 'coreRef' } }, { functions: { fromTop: 'shadowed' } }), + }; + const resolved = resolveArtifactCollections(artifact) as Record; + expect(Object.keys(resolved.functions).sort()).toEqual(['fromCore', 'fromTop']); + expect(typeof resolved.functions.fromTop).toBe('function'); + }); + + it('leaves a key NO source declares ABSENT, never `[]`', () => { + const optionB = { packages: packagesOf({ objects: [obj('account')] }) }; + const resolved = resolveArtifactCollections(optionB) as Record; + expect('objects' in resolved).toBe(true); + expect('permissions' in resolved).toBe(false); + expect('jobs' in resolved).toBe(false); + }); + + it('raises the load path\'s OWN refusal for a malformed `packages[]`', () => { + // ADR-0112 envelope, from `resolveArtifactPackageOrder` — the same + // refusal `ObjectQLPlugin`'s `manifest` service raises on these bytes. + // Resolving collections out of an artifact the loader would refuse is + // not a quieter outcome, it is a different answer to what it contains. + let raised: any; + try { + resolveArtifactCollections({ packages: [{ id: 'inlined-not-wrapped' }] }); + } catch (err) { + raised = err; + } + expect(raised?.code).toBe('INVALID_ARTIFACT_PACKAGE_ENTRY'); + expect(raised?.status).toBe(422); + }); +}); diff --git a/packages/core/src/artifact-collections.ts b/packages/core/src/artifact-collections.ts new file mode 100644 index 0000000000..963dbc4b42 --- /dev/null +++ b/packages/core/src/artifact-collections.ts @@ -0,0 +1,284 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * ADR-0130 D4 / option B — reading a top-level COLLECTION out of an artifact + * that may carry it flattened, under `packages[]`, or both. + * + * ## The problem this exists for + * + * A multi-package artifact serializes every definition TWICE today: once + * flattened to the artifact's top level, once inside `packages[i].manifest` + * (`composeStacks(…, { manifest: 'preserve' })` is ADDITIVE — see + * `assemblePackageBody` in `packages/spec/src/stack.zod.ts`). Option B, ruled on + * #14512 comment 5528589044 (maintainer 2026-09-03, decision batch #23), + * removes the flattened copy so `packages[]` carries each definition exactly + * once — READERS FIRST, emitter last. + * + * Every reader that says `artifact.` and nothing else therefore has + * a deadline, and its failure mode is SILENT: nothing throws, the collection is + * simply absent, so the artifact boots clean having lost its declarative + * actions, its scheduled jobs, its seed data or its default permission set. + * #15004's acceptance pin (`packages/cli/test/option-b-reader-acceptance.pin.test.ts`) + * measured 24 such subsystems. + * + * ## What this module is, and why it is ONE function + * + * The enumeration missed reader sites twice (#14512 comments 5523603341 and + * 5523741937). N readers each growing their own `packages[]` walk is that same + * miss with a longer tail: two walks that ordered or de-duplicated differently + * would disagree about what an artifact CONTAINS, not merely about the order. + * So this is the one resolution, in the package all three reader cards + * (#15005 runtime · #15006 cli · #15007 plugin-security) already depend on, and + * it sits beside `resolveArtifactPackageOrder` because it is built out of it. + * + * ⛔ Ordering is NOT re-derived here. `resolveArtifactPackageOrder` (ADR-0130 + * D4/D5, the platform's one artifact package sorter, reused by + * `ObjectQLPlugin`'s manifest service and by `MetadataPlugin`'s artifact door) + * decides both which entries are admissible and what order they register in. + * + * ## The merge rule, and why it is top-level-first + * + * For each collection key: + * + * 1. the artifact's own top-level value is taken FIRST, whole and unchanged; + * 2. every package body then contributes, in package order, the items the top + * level did not already claim. + * + * Two properties fall out of that order, and both are load-bearing: + * + * - **On today's additive artifact the result is what the reader sees now.** + * The flattened top level carries every definition, so step 2 contributes + * nothing and the array is the same array, in the same order, with the same + * element references. This is the D7 posture the whole reader program + * depends on: the readers change while the artifact does not, so a + * regression on the shape the platform emits TODAY is not a risk this + * change takes. + * - **On an option-B artifact the result is package order**, because the top + * level contributes nothing and step 2 is the whole answer. + * + * A partially-flattened artifact — the real transition state, and the case a + * "use the top level, else `packages[]`" fallback would get wrong — is + * resolved per key and per item rather than per artifact. + * + * ## What "the top level already claimed it" means + * + * Identity is the item's `name` when it has one, and a stable serialization of + * the item otherwise (`datasourceMapping` rules, `translations` bundles, seed + * `data` datasets and `requires` entries carry no name). Both spellings are + * needed and neither alone is enough: + * + * - Structural identity alone breaks on `objects`, the one collection + * `composeStacks` MERGES rather than concatenates: a base and an extension + * of the same object are one merged entry at the top level and two separate + * bodies under `packages[]`, so the two copies do not serialize alike and + * the merged top level would be joined by both unmerged halves. + * - Name identity alone would drop the second of two same-named entries + * under `packages[]` — which is exactly what a base and its extension are + * on an option-B artifact, where nothing merged them. + * + * Package bodies deliberately do NOT claim against each other: only the top + * level claims. Two packages contributing an identical `requires` entry + * concatenate on the additive shape (`COMPOSE_KEY_DISPOSITIONS`), and they + * concatenate here too, so the two shapes agree. + * + * ## The key set is DERIVED + * + * `ObjectStackDefinitionSchema` ∩ `AssembledPackageBodySchema` is precisely + * "the collections a package owns" — the second schema derives its own key set + * from `COMPOSE_KEY_DISPOSITIONS`, which is total over the stack schema. A + * transcribed list here would be a third copy of that set and would fail in the + * silent direction: a collection family added next month would simply never be + * resolved out of `packages[]`, and the artifact would boot clean without it. + * (#14877 is to publish this key set as an export; when it lands, this module + * reads it instead of deriving it and nothing else here changes.) + */ + +import { AssembledPackageBodySchema, ObjectStackDefinitionSchema } from '@objectstack/spec'; + +import { resolveArtifactPackageOrder } from './artifact-packages.js'; + +/** A Zod object schema, read for its declared key set only. */ +type KeyedShape = { shape: Record }; + +const shapeKeys = (schema: unknown): string[] => Object.keys((schema as KeyedShape).shape); + +let cachedCollectionKeys: readonly string[] | undefined; + +/** + * Every top-level key of an `ObjectStackDefinition` that a PACKAGE can own — + * i.e. every key an option-B artifact carries under `packages[i].manifest` + * instead of at its top level. + * + * Derived (see the module header), and memoized: the derivation forces both + * schemas' shapes, and the artifacts that reach it are multi-package ones whose + * entries `resolveArtifactPackageOrder` has already parsed against + * `ArtifactPackageSchema` — which embeds `AssembledPackageBodySchema` — so the + * shapes are built by the time the first call needs them. + */ +export function packageOwnedCollectionKeys(): readonly string[] { + if (cachedCollectionKeys !== undefined) return cachedCollectionKeys; + const owned = new Set(shapeKeys(AssembledPackageBodySchema)); + cachedCollectionKeys = Object.freeze(shapeKeys(ObjectStackDefinitionSchema).filter((k) => owned.has(k))); + return cachedCollectionKeys; +} + +/** + * A deterministic serialization of `value`, with object keys sorted so two + * copies of one definition that differ only in key order still compare equal. + * + * Never throws: a cycle serializes as `"[circular]"` rather than raising, and a + * callable as `"[function]"`. Both matter because this runs on the boot path + * over a FROM-SOURCE config as well as over parsed artifact JSON, and a throw + * here would turn "the reader could not tell two copies apart" into "the app + * does not boot". Callables are compared by reference first (see + * {@link claimedIdentities}), so collapsing them here costs nothing. + */ +function stableIdentity(value: unknown): string { + const seen = new WeakSet(); + const encode = (v: unknown): unknown => { + if (typeof v === 'function') return '[function]'; + if (typeof v === 'bigint') return `${v}n`; + if (v === null || typeof v !== 'object') return v; + if (seen.has(v as object)) return '[circular]'; + seen.add(v as object); + if (Array.isArray(v)) return v.map(encode); + const out: Record = {}; + for (const key of Object.keys(v as Record).sort()) { + out[key] = encode((v as Record)[key]); + } + return out; + }; + try { + return JSON.stringify(encode(value)) ?? 'undefined'; + } catch { + // Unreachable through `encode` above, which resolves cycles and drops + // callables — kept because the alternative to a wrong answer here is a + // boot that dies inside a de-duplication helper. + return '[unserializable]'; + } +} + +/** How one collection item is recognized as "already present". */ +function itemIdentity(item: unknown): string { + if (item !== null && typeof item === 'object') { + const named = (item as { name?: unknown }).name; + if (typeof named === 'string' && named.length > 0) return `name:${named}`; + } + return `value:${stableIdentity(item)}`; +} + +/** The identities (and object references) one top-level collection claims. */ +function claimedIdentities(items: readonly unknown[]): { + has: (item: unknown) => boolean; +} { + const refs = new WeakSet(); + const ids = new Set(); + for (const item of items) { + if (item !== null && typeof item === 'object') refs.add(item as object); + ids.add(itemIdentity(item)); + } + return { + has: (item: unknown) => + (item !== null && typeof item === 'object' && refs.has(item as object)) || ids.has(itemIdentity(item)), + }; +} + +/** True for a value that carries collection items as a `name -> entry` record. */ +const isRecord = (v: unknown): v is Record => + v !== null && typeof v === 'object' && !Array.isArray(v); + +/** + * Merge one collection key across the artifact's top level and its package + * bodies, per the module header's rule. + * + * @returns The merged value, or `undefined` when NO source declares the key — + * in which case the caller leaves the key ABSENT rather than writing an empty + * one. That distinction is read downstream: `createStandaloneStack` omits + * `objects` entirely when the artifact declares none, and consumers of that + * result gate on the key's presence. + */ +function mergeCollection(top: unknown, fromBodies: readonly unknown[]): unknown { + const contributions = [top, ...fromBodies].filter((v) => v !== undefined && v !== null); + if (contributions.length === 0) return undefined; + + // The first declared contribution decides the shape. `datasources` is + // legitimately either an array or a `name -> definition` record (AppPlugin + // reads both), and `functions` is a record; mixing the two spellings inside + // one artifact is not a shape this merges — the first one wins and the rest + // of that kind join it, which is what the pre-option-B reader did with the + // one copy it had. + if (Array.isArray(contributions[0])) { + const base = Array.isArray(top) ? top : []; + const claimed = claimedIdentities(base); + const out = [...base]; + for (const contribution of fromBodies) { + if (!Array.isArray(contribution)) continue; + for (const item of contribution) { + if (claimed.has(item)) continue; + out.push(item); + } + } + // Identity when nothing was added: the caller keeps the artifact's own + // array, references included, so `{ ...artifact }` stays a cheap + // reference copy on every additive artifact. + return out.length === base.length && Array.isArray(top) ? top : out; + } + + if (isRecord(contributions[0])) { + const out: Record = {}; + for (const contribution of contributions) { + if (!isRecord(contribution)) continue; + for (const [key, value] of Object.entries(contribution)) { + if (!(key in out)) out[key] = value; + } + } + return isRecord(top) && Object.keys(out).length === Object.keys(top).length ? top : out; + } + + // A scalar collection value is not a shape any collection key declares; + // hand back what the artifact carried rather than inventing a merge. + return top !== undefined && top !== null ? top : contributions[0]; +} + +/** + * Read `artifact` with every package-owned collection resolved across BOTH + * artifact shapes — the flattened top level and `packages[]` (ADR-0130 D4). + * + * The returned value is a shallow copy whose envelope keys (`manifest`, `api`, + * `server`, `i18n`, `runtimeModule`, `onEnable`, `packages`) are the caller's + * own references, so it is a drop-in for the artifact at any read site. + * + * ⚠️ Returns the ARGUMENT ITSELF, unchanged, for anything that does not carry a + * `packages` array — which is every single-package artifact and every + * `defineStack()` config the platform has ever booted. That 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. + * + * @throws The ADR-0112 refusal `resolveArtifactPackageOrder` raises for a + * malformed `packages[]` entry, a package with no usable id, or a duplicate + * package — the same refusal `ObjectQLPlugin`'s `manifest` service already + * raises on the same artifact during boot. Resolving collections out of an + * artifact the loader would refuse is not a quieter outcome, it is a + * different answer to what the artifact contains. + */ +export function resolveArtifactCollections(artifact: T): T { + if (artifact === null || typeof artifact !== 'object') return artifact; + if (!Array.isArray((artifact as { packages?: unknown }).packages)) return artifact; + + const bodies = resolveArtifactPackageOrder(artifact) as Array | null | undefined>; + const source = artifact as unknown as Record; + let resolved: Record | undefined; + + for (const key of packageOwnedCollectionKeys()) { + const merged = mergeCollection( + source[key], + bodies.map((body) => (body !== null && typeof body === 'object' ? body[key] : undefined)), + ); + if (merged === source[key]) continue; + if (merged === undefined) continue; + resolved ??= { ...source }; + resolved[key] = merged; + } + + return (resolved ?? artifact) as T; +} diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index 7e9d3eb3f0..a41c2adda7 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -17,6 +17,14 @@ export * from './plugin-order.js'; // because the ordering it performs is `resolvePluginOrder` directly above. // `@objectstack/objectql` re-exports it, so its published surface is unchanged. export * from './artifact-packages.js'; +// ADR-0130 D4 / option B (#15005) — the ONE way to read a top-level +// collection out of an artifact that may carry it flattened, under +// `packages[]`, or both. Beside `artifact-packages.js` and for the same +// reason: its readers live in packages that cannot import each other +// (`@objectstack/runtime`, `@objectstack/cli`, `@objectstack/plugin-security`), +// and N private `packages[]` walks would disagree about what an artifact +// CONTAINS rather than merely about the order. +export * from './artifact-collections.js'; export * from './lite-kernel.js'; export * from './types.js'; export * from './logger.js'; diff --git a/packages/runtime/src/app-plugin.option-b-packages.test.ts b/packages/runtime/src/app-plugin.option-b-packages.test.ts new file mode 100644 index 0000000000..356d1a34de --- /dev/null +++ b/packages/runtime/src/app-plugin.option-b-packages.test.ts @@ -0,0 +1,211 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #15005 — `@objectstack/runtime` reads its collections out of `packages[]` + * (ADR-0130 D4 / option B, ruled on #14512 comment 5528589044). + * + * The program's acceptance pin lives one package away + * (`packages/cli/test/option-b-reader-acceptance.pin.test.ts`, #15004) because + * it has to drive `@objectstack/cli` and `@objectstack/plugin-security` too. + * This file is the runtime side's OWN regression cover, and it pins the two + * things that pin cannot separate: + * + * - the option-B shape is READ (a multi-package artifact whose collections + * live only under `packages[]` reaches every collector and the scheduler); + * - today's ADDITIVE shape is read exactly ONCE. That direction is the risk + * this change actually takes: `composeStacks(…, { manifest: 'preserve' })` + * emits every definition twice — flattened AND under `packages[]` — so a + * reader that simply concatenated both would register every action, hook, + * job and seed dataset twice on every multi-package artifact shipping + * today, and the acceptance pin's rows (`length > 0`) would not notice. + */ + +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import type { PluginContext } from '@objectstack/core'; + +import { + AppPlugin, + collectBundleActions, + collectBundleFunctionEntries, + collectBundleHooks, +} from './app-plugin.js'; + +const field = { name: 'name', type: 'text', label: 'Name' } as const; + +/** The App half: an object carrying an EMBEDDED action, plus a seed dataset. */ +const coreBody = () => ({ + id: 'com.test.optionb.core', + name: 'Option-B Core', + version: '1.0.0', + type: 'app', + objects: [{ + name: 'ob_account', + label: 'Account', + fields: { name: field }, + actions: [{ + name: 'ob_object_action', + label: 'Object Action', + objectName: 'ob_account', + type: 'script', + body: { language: 'js', source: 'return 1;' }, + }], + }], + data: [{ object: 'ob_account', mode: 'upsert', externalId: 'name', records: [{ name: 'seeded' }] }], +}); + +/** The module half: a global action, a hook, a declared function and a job. */ +const ordersBody = () => ({ + id: 'com.test.optionb.orders', + name: 'Option-B Orders', + version: '1.0.0', + type: 'module', + dependencies: { 'com.test.optionb.core': '^1.0.0' }, + objects: [{ name: 'ob_order', label: 'Order', fields: { name: field } }], + actions: [{ name: 'ob_global_action', label: 'Global Action', type: 'script', body: { language: 'js', source: 'return 1;' } }], + hooks: [{ + name: 'ob_before_insert', + label: 'Before Insert', + object: 'ob_order', + events: ['beforeInsert'], + body: { language: 'js', source: 'return;' }, + }], + jobs: [{ name: 'ob_nightly', label: 'Nightly', schedule: { type: 'cron', expression: '0 3 * * *', timezone: 'UTC' }, handler: 'obSweep' }], +}); + +const manifest = { id: 'com.test.optionb.core', name: 'Option-B Core', version: '1.0.0', type: 'app' }; + +/** `packages[]` carries every definition once; the top level carries none. */ +const optionBBundle = (extra: Record = {}) => ({ + manifest, + packages: [{ manifest: ordersBody() }, { manifest: coreBody() }], + ...extra, +}); + +/** + * What the platform emits TODAY: the flattened top level AND `packages[]`, from + * the same definitions. Built by flattening the same bodies, so a reader that + * counted both copies reads double here. + */ +const additiveBundle = (extra: Record = {}) => { + const core = coreBody(); + const orders = ordersBody(); + return { + manifest, + objects: [...orders.objects, ...core.objects], + actions: [...orders.actions], + hooks: [...orders.hooks], + jobs: [...orders.jobs], + data: [...core.data], + packages: [{ manifest: orders }, { manifest: core }], + ...extra, + }; +}; + +describe('#15005 — runtime collectors resolve `packages[]`', () => { + it('reads actions — global AND object-embedded — out of `packages[]`', () => { + const names = collectBundleActions(optionBBundle()).map((a) => `${a.object ?? 'global'}:${a.name}`); + // The embedded one is the widening #14512 comment 5523603341 measured: + // it rides on `objects[]`, so it disappears with the top-level objects + // array rather than with `actions`. + expect(names.sort()).toEqual(['global:ob_global_action', 'ob_account:ob_object_action']); + }); + + it('reads hooks and declared functions out of `packages[]`', () => { + expect(collectBundleHooks(optionBBundle()).map((h) => h.name)).toEqual(['ob_before_insert']); + const declared = { obSweep: { handler: () => undefined, effect: 'writes' } }; + const entries = collectBundleFunctionEntries( + optionBBundle({ packages: [{ manifest: { ...ordersBody(), functions: declared } }, { manifest: coreBody() }] }), + ); + // The DECLARATION survives, not just the callable: an entry read back as + // `effect: 'pure'` is #4396's silent un-declaring of a writer. + expect(entries.obSweep?.effect).toBe('writes'); + }); + + it('reads today\'s ADDITIVE artifact exactly once — no doubling', () => { + expect(collectBundleActions(additiveBundle())).toHaveLength(2); + expect(collectBundleHooks(additiveBundle())).toHaveLength(1); + }); + + it('is unchanged for a single-package bundle', () => { + const single = { manifest, actions: [{ name: 'solo', label: 'Solo', type: 'script' }] }; + expect(collectBundleActions(single).map((a) => a.name)).toEqual(['solo']); + expect(collectBundleHooks(single)).toEqual([]); + }); +}); + +describe('#15005 — AppPlugin schedules and hands out `packages[]` collections', () => { + let scheduled: Array<{ name: string; run: (c: unknown) => Promise }>; + let readyHooks: Array<() => Promise>; + let ctx: PluginContext; + let mappingRules: unknown[]; + + beforeEach(() => { + scheduled = []; + readyHooks = []; + mappingRules = []; + ctx = { + logger: { info: vi.fn(), error: vi.fn(), warn: vi.fn(), debug: vi.fn() }, + registerService: vi.fn(), + getService: vi.fn((name: string) => { + if (name === 'job') { + return { + schedule: async (jobName: string, _s: unknown, run: (c: unknown) => Promise) => { + scheduled.push({ name: jobName, run }); + return { id: jobName }; + }, + }; + } + if (name === 'objectql') { + return { setDatasourceMapping: (rules: unknown[]) => { mappingRules = rules; } }; + } + return undefined; + }), + getServices: vi.fn(() => []), + hook: vi.fn((event: string, cb: () => Promise) => { + if (event === 'kernel:ready') readyHooks.push(cb); + }), + trigger: vi.fn(), + } as unknown as PluginContext; + }); + + const fireReady = async () => { for (const cb of readyHooks) await cb(); }; + + it('schedules a job declared in a package body, and its handler context carries the RESOLVED bundle', async () => { + const sweep = vi.fn(async () => undefined); + const plugin = new AppPlugin(optionBBundle({ + packages: [ + { manifest: { ...ordersBody(), functions: { obSweep: sweep } } }, + { manifest: coreBody() }, + ], + })); + + await plugin.start!(ctx); + await fireReady(); + + expect(scheduled.map((s) => s.name)).toEqual(['ob_nightly']); + + // #14094 hands the handler `ctx.bundle` as its data reach. Reading + // `bundle.objects` off the RAW option-B artifact answers `undefined` + // with nothing thrown, so the resolved view is what is handed over. + let handed: any; + sweep.mockImplementation(async (jobCtx: any) => { handed = jobCtx.bundle; return undefined; }); + await scheduled[0].run({}); + expect(sweep).toHaveBeenCalledTimes(1); + // Package order (`core` before `orders`, which depends on it), from + // `resolveArtifactPackageOrder` — not the array's own order. + expect(handed.objects.map((o: any) => o.name)).toEqual(['ob_account', 'ob_order']); + // Envelope keys stay the caller's own references. + expect(handed.manifest).toBe(manifest); + }); + + it('routes objects with a `datasourceMapping` declared in a package body', async () => { + const rules = [{ datasource: 'ob_primary', default: true }]; + const plugin = new AppPlugin(optionBBundle({ + packages: [{ manifest: ordersBody() }, { manifest: { ...coreBody(), datasourceMapping: rules } }], + })); + + await plugin.start!(ctx); + + expect(mappingRules).toEqual(rules); + }); +}); diff --git a/packages/runtime/src/app-plugin.ts b/packages/runtime/src/app-plugin.ts index 76c4259682..1cb519eddb 100644 --- a/packages/runtime/src/app-plugin.ts +++ b/packages/runtime/src/app-plugin.ts @@ -1,6 +1,6 @@ // Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. -import { Plugin, PluginContext, wireAuthoredTranslationSync } from '@objectstack/core'; +import { Plugin, PluginContext, resolveArtifactCollections, wireAuthoredTranslationSync } from '@objectstack/core'; import { applyArtifactForwardConversions, assertProtocolCompat } from '@objectstack/metadata-core'; import { resolveTenancyPosture } from '@objectstack/types'; import { postureEnforcesWall, type TenancyPosture } from '@objectstack/spec/security'; @@ -113,6 +113,8 @@ export class AppPlugin implements Plugin { requiresServices: string[] = ['manifest']; private bundle: any; + /** Memoized backing store for {@link collections}. */ + private resolvedCollections?: any; private projectContext?: AppPluginProjectContext; /** * The context handed to `init()`, retained so `destroy()` can emit the @@ -139,6 +141,35 @@ export class AppPlugin implements Plugin { */ readonly securityMetadataRegistrar: AppPluginSecurityMetadataRegistrar; + /** + * `this.bundle` with every package-owned collection resolved across BOTH + * artifact shapes — the flattened top level and `packages[]` (ADR-0130 D4 / + * option B, #15005; the resolution itself is + * `resolveArtifactCollections` in `@objectstack/core`, shared with the + * other readers of the same key). + * + * ⛔ Read COLLECTIONS through this, never `this.bundle` — `this.bundle` is + * what the caller handed us and, on a multi-package option-B artifact, its + * `objects` / `jobs` / `data` / `translations` / `datasources` / + * `datasourceMapping` / `permissions` / `positions` are simply ABSENT while + * `packages[]` carries every one of them. Nothing throws on that path: the + * subsystem is handed an empty list and the app boots having lost the + * collection (#15004 measured 24 such subsystems). + * + * ENVELOPE keys stay on `this.bundle`, and the two are the same object + * whenever the bundle carries no `packages[]` — which is every + * single-package artifact and every `defineStack()` config — so this + * accessor cannot move the shape that ships today. + * + * Lazy so construction stays free of the ADR-0112 refusal a malformed + * `packages[]` raises: that refusal belongs to the boot, where the + * `manifest` service already raises it on the same bytes, not to `new + * AppPlugin(...)`. + */ + private get collections(): any { + return (this.resolvedCollections ??= resolveArtifactCollections(this.bundle)); + } + constructor( bundle: any, projectContext?: AppPluginProjectContext, @@ -533,12 +564,13 @@ export class AppPlugin implements Plugin { ctx.logger.debug('Retrieved ObjectQL engine service', { appId }); // Configure datasourceMapping if provided in the stack definition - if (this.bundle.datasourceMapping && Array.isArray(this.bundle.datasourceMapping)) { + const datasourceMapping = this.collections.datasourceMapping; + if (datasourceMapping && Array.isArray(datasourceMapping)) { ctx.logger.info('Configuring datasource mapping rules', { appId, - ruleCount: this.bundle.datasourceMapping.length + ruleCount: datasourceMapping.length }); - ql.setDatasourceMapping(this.bundle.datasourceMapping); + ql.setDatasourceMapping(datasourceMapping); } // Surface code-defined datasources (ADR-0015 Addendum) in the metadata @@ -556,7 +588,7 @@ export class AppPlugin implements Plugin { // reject at load, loudly (outside the lenient catch below), instead of // letting the collision produce undefined routing. { - const dsDefs = this.bundle.datasources; + const dsDefs = this.collections.datasources; const declared = Array.isArray(dsDefs) ? dsDefs : dsDefs && typeof dsDefs === 'object' @@ -574,7 +606,7 @@ export class AppPlugin implements Plugin { } } try { - const dsDefs = this.bundle.datasources; + const dsDefs = this.collections.datasources; const dsList = Array.isArray(dsDefs) ? dsDefs : dsDefs && typeof dsDefs === 'object' @@ -616,7 +648,7 @@ export class AppPlugin implements Plugin { // so the kernel's init-all-then-start-all ordering guarantees the // connection service was already registered during init. try { - const dsDefs = this.bundle.datasources; + const dsDefs = this.collections.datasources; const dsList: any[] = Array.isArray(dsDefs) ? dsDefs : dsDefs && typeof dsDefs === 'object' @@ -643,7 +675,7 @@ export class AppPlugin implements Plugin { connection = undefined; } if (typeof connection?.connectDeclared === 'function') { - const objects = Array.isArray(this.bundle.objects) ? this.bundle.objects : []; + const objects = Array.isArray(this.collections.objects) ? this.collections.objects : []; const results = await connection.connectDeclared({ datasources: dsList, objects, @@ -757,9 +789,9 @@ export class AppPlugin implements Plugin { }, ); } else { - const rawSecurityBundle: any = this.bundle.manifest - ? { ...this.bundle.manifest, ...this.bundle } - : this.bundle; + const rawSecurityBundle: any = this.collections.manifest + ? { ...this.collections.manifest, ...this.collections } + : this.collections; // [#12844] Same bytes, same conversion policy — one funnel. // // A `defineStack()` module can declare an `engines.protocol` @@ -969,8 +1001,8 @@ export class AppPlugin implements Plugin { // resolved through `collectBundleFunctions(bundle)` — the same // registry used by hooks/actions, keeping the surface uniform. try { - const jobs: any[] = Array.isArray(this.bundle.jobs) - ? this.bundle.jobs + const jobs: any[] = Array.isArray(this.collections.jobs) + ? this.collections.jobs : Array.isArray((this.bundle.manifest || {}).jobs) ? (this.bundle.manifest as any).jobs : []; @@ -1029,7 +1061,14 @@ export class AppPlugin implements Plugin { const jobContext: JobHandlerContext = { ...jobCtx, jobId: jobName, - bundle: this.bundle, + // The RESOLVED view, not `this.bundle`: + // a handler reading `ctx.bundle.objects` + // on a multi-package option-B artifact + // would otherwise read `undefined` with + // nothing thrown (ADR-0130 D4, #15005). + // Identical reference on every bundle + // that carries no `packages[]`. + bundle: this.collections, ql, logger: ctx.logger, }; @@ -1107,8 +1146,8 @@ export class AppPlugin implements Plugin { const seedDatasets: any[] = []; // 1. Top-level `data` field (new standard location on ObjectStackDefinition) - if (Array.isArray(this.bundle.data)) { - seedDatasets.push(...this.bundle.data); + if (Array.isArray(this.collections.data)) { + seedDatasets.push(...this.collections.data); } // 2. Legacy: `manifest.data` (backward compatibility) @@ -1555,7 +1594,7 @@ export class AppPlugin implements Plugin { if (this.organizationWallActive(ctx)) return; const knownObjects = new Set( - (Array.isArray(this.bundle.objects) ? this.bundle.objects : []) + (Array.isArray(this.collections.objects) ? this.collections.objects : []) .map((o: any) => o?.name) .filter((n: any): n is string => typeof n === 'string'), ); @@ -1698,11 +1737,11 @@ export class AppPlugin implements Plugin { // Collect translation bundles early to determine if we have data const bundles: Array> = []; - if (Array.isArray(this.bundle.translations)) { - bundles.push(...this.bundle.translations); + if (Array.isArray(this.collections.translations)) { + bundles.push(...this.collections.translations); } const manifest = this.bundle.manifest || this.bundle; - if (manifest && Array.isArray(manifest.translations) && manifest.translations !== this.bundle.translations) { + if (manifest && Array.isArray(manifest.translations) && manifest.translations !== this.collections.translations) { bundles.push(...manifest.translations); } @@ -1838,8 +1877,21 @@ export class AppPlugin implements Plugin { // some legacy bundles still nest them under `manifest.hooks`. We dedupe // (by reference) so the same array isn't bound twice when both shapes // happen to point at the same list. +// +// [ADR-0130 D4 / option B, #15005] Each collector below reads its collection +// through `resolveArtifactCollections` FIRST, so a multi-package artifact that +// carries the collection under `packages[]` instead of flattened at the top +// level is read rather than silently seen as empty. The resolution is shared +// (`@objectstack/core`) rather than open-coded per collector: three walks of +// one `packages[]` that ordered or de-duplicated differently would disagree +// about what the artifact CONTAINS. On any bundle without `packages[]` it +// returns the argument itself, so these three functions are unchanged for every +// single-package artifact and every `defineStack()` config. -/** Collect declarative `Hook` definitions from a bundle (top-level + manifest). */ +/** + * Collect declarative `Hook` definitions from a bundle — top-level, + * `manifest.hooks`, and (ADR-0130 D4) every package body's `hooks`. + */ export function collectBundleHooks(bundle: any): any[] { const out: any[] = []; const seen = new Set(); @@ -1852,8 +1904,9 @@ export function collectBundleHooks(bundle: any): any[] { } } }; - push(bundle?.hooks); - push(bundle?.manifest?.hooks); + const stack = resolveArtifactCollections(bundle) as any; + push(stack?.hooks); + push(stack?.manifest?.hooks); return out; } @@ -1893,13 +1946,18 @@ export function collectBundleActions( out.push(inferredObject ? { ...a, object: inferredObject } : { ...a }); } }; - push(bundle?.actions); - push(bundle?.manifest?.actions); - if (Array.isArray(bundle?.objects)) { - for (const o of bundle.objects) push(o?.actions, o?.name); + // Both walks read the RESOLVED stack: an option-B artifact loses the + // object-embedded actions with the top-level `objects` array that carried + // them, which is the widening #14512 comment 5523603341 measured (4 -> 0, + // every declarative action 404-ing at dispatch). + const stack = resolveArtifactCollections(bundle) as any; + push(stack?.actions); + push(stack?.manifest?.actions); + if (Array.isArray(stack?.objects)) { + for (const o of stack.objects) push(o?.actions, o?.name); } - if (Array.isArray(bundle?.manifest?.objects)) { - for (const o of bundle.manifest.objects) push(o?.actions, o?.name); + if (Array.isArray(stack?.manifest?.objects)) { + for (const o of stack.manifest.objects) push(o?.actions, o?.name); } return out; } @@ -1935,8 +1993,9 @@ export function collectBundleFunctionEntries(bundle: any): Record', effect }]`) carries its // declaration exactly like the map form, but names itself by an entry's // `name` instead of by a map key. Rebuilding it as a map below would @@ -147,10 +162,10 @@ export async function mergeRuntimeModule(bundle: any, artifactAbsPath: string, t // writes are still counted as none, so #4354's broken-sweep alert stays // quiet on the one run that needed it. Unreachable until #6238 let the // array form past the parse; reachable now, so it is handled here. - if (Array.isArray(bundle.functions)) { + if (Array.isArray(declaredFunctions)) { const moduleFns = fns as Record; const attached = new Set(); - const mergedEntries = (bundle.functions as unknown[]).map((entry) => { + const mergedEntries = (declaredFunctions as unknown[]).map((entry) => { if (!entry || typeof entry !== 'object' || Array.isArray(entry)) return entry; const record = entry as Record; const name = typeof record.name === 'string' ? record.name : undefined; @@ -170,8 +185,8 @@ export async function mergeRuntimeModule(bundle: any, artifactAbsPath: string, t bundle.functions = mergedEntries; return; } - const existing = (bundle.functions && typeof bundle.functions === 'object') - ? bundle.functions as Record + const existing = (declaredFunctions && typeof declaredFunctions === 'object') + ? declaredFunctions as Record : {}; // The module supplies the CALLABLE; the JSON supplies what the function // DECLARED about itself (`{ handler: '', effect: 'writes' }`, diff --git a/packages/runtime/src/resolve-project-database.ts b/packages/runtime/src/resolve-project-database.ts index 8613c5d9f6..6114c33d26 100644 --- a/packages/runtime/src/resolve-project-database.ts +++ b/packages/runtime/src/resolve-project-database.ts @@ -44,6 +44,7 @@ import { resolve as resolvePath, isAbsolute } from 'node:path'; import { existsSync, readFileSync } from 'node:fs'; import { homedir } from 'node:os'; +import { resolveArtifactCollections } from '@objectstack/core'; import { resolveDatabaseDriverId, resolveDriverId } from '@objectstack/spec/data'; /** The unified default database filename (`/data/objectstack.db`). */ @@ -192,7 +193,22 @@ function readConfigDeclaredDefault(opts: { try { const parsed = JSON.parse(readFileSync(artifactPath, 'utf8')); // Same envelope unwrap as `loadArtifactBundle` (`{ schemaVersion, metadata }`). - bundle = parsed?.schemaVersion != null && parsed?.metadata !== undefined ? parsed.metadata : parsed; + const unwrapped = parsed?.schemaVersion != null && parsed?.metadata !== undefined ? parsed.metadata : parsed; + // [ADR-0130 D4 / option B, #15005] …and the same collection resolution + // every other reader of this artifact makes. This tier runs 112 lines + // BEFORE `loadArtifactBundle` inside `createStandaloneStack`, and is + // reached independently of any stack at all from `os dev`, `os start` + // and `os db clean` — so "the bundle path was taught" says nothing + // about it. Without this, a multi-package artifact whose + // `datasourceMapping` and `datasources` live under `packages[]` + // resolves NO declared default and falls through to the unified + // default database: the project boots, silently, against the wrong file. + // + // Inside this `try` on purpose — a malformed `packages[]` raises the + // ADR-0112 refusal `resolveArtifactPackageOrder` owns, and this + // resolver's posture for an artifact it cannot read is to DECLINE (the + // boot's own loader reports that loudly), never to brick a URL lookup. + bundle = resolveArtifactCollections(unwrapped); } catch { // Unreadable/malformed artifact — the boot's own loader reports that // loudly; the URL resolver just cannot consult it. diff --git a/packages/runtime/src/standalone-stack.ts b/packages/runtime/src/standalone-stack.ts index e6a29257ef..31249428d2 100644 --- a/packages/runtime/src/standalone-stack.ts +++ b/packages/runtime/src/standalone-stack.ts @@ -57,6 +57,7 @@ import { mkdirSync, existsSync } from 'node:fs'; import { homedir } from 'node:os'; import { z } from 'zod'; import { stampSearchPinyinEnabled } from '@objectstack/types'; +import { resolveArtifactCollections } from '@objectstack/core'; import { BUILTIN_DRIVER_IDS, DATABASE_DRIVER_SELECTION_ALIASES, @@ -777,20 +778,34 @@ export async function createStandaloneStack(config?: StandaloneStackConfig): Pro // and driver auto-registration. We copy *references* — no clone — so // the caller can `{ ...originalConfig, ...standaloneStack }` without // double-merging large object arrays. + // + // [ADR-0130 D4 / option B, #15005] Read through the RESOLVED stack, never + // `artifactBundle` directly: on a multi-package artifact that carries these + // collections under `packages[]` the top level is ABSENT, and every key + // below would be silently omitted from the result — taking the CLI's tier + // resolution, its engine/driver auto-registration gates and the ADR-0056 D7 + // default permission set with it, with nothing thrown. Identical reference + // (so identical output) for every artifact without `packages[]`. + // + // ⚠️ This is one of the TWO boundaries feeding the D7 permission surface. + // The other is the from-source config path (`appSecurityPluginOptions` in + // `@objectstack/cli` / `@objectstack/plugin-security`, cards #15006 / #15007) + // — fixing one side leaves the other empty. + const artifactStack: any = artifactBundle ? resolveArtifactCollections(artifactBundle) : artifactBundle; const requires: string[] | undefined = - Array.isArray(artifactBundle?.requires) - ? (artifactBundle.requires.filter((c: unknown) => typeof c === 'string') as string[]) + Array.isArray(artifactStack?.requires) + ? (artifactStack.requires.filter((c: unknown) => typeof c === 'string') as string[]) : undefined; const objects: any[] | undefined = - Array.isArray(artifactBundle?.objects) ? artifactBundle.objects : undefined; + Array.isArray(artifactStack?.objects) ? artifactStack.objects : undefined; const manifest: any | undefined = artifactBundle?.manifest; // ADR-0056 D7 — surface app-declared RBAC so the CLI's artifact-serve // path honours an `isDefault` profile (appDefaultPermissionSetName) and // registers application org names, exactly like the config-load path. const permissions: any[] | undefined = - Array.isArray(artifactBundle?.permissions) ? artifactBundle.permissions : undefined; + Array.isArray(artifactStack?.permissions) ? artifactStack.permissions : undefined; const positions: any[] | undefined = - Array.isArray(artifactBundle?.positions) ? artifactBundle.positions : undefined; + Array.isArray(artifactStack?.positions) ? artifactStack.positions : undefined; const i18n: any | undefined = artifactBundle?.i18n && typeof artifactBundle.i18n === 'object' ? artifactBundle.i18n : undefined; From eb7d10ddab8713c81ad6070fab4afeff09ed2ce0 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 4 Sep 2026 06:19:38 +0000 Subject: [PATCH 2/7] chore(changeset): the option-B reader half for @objectstack/core and @objectstack/runtime Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01UHvF5hyiZjnCyExFnfQB8m --- .../artifact-packages-collection-reads.md | 55 +++++++++++++++++++ 1 file changed, 55 insertions(+) create mode 100644 .changeset/artifact-packages-collection-reads.md diff --git a/.changeset/artifact-packages-collection-reads.md b/.changeset/artifact-packages-collection-reads.md new file mode 100644 index 0000000000..0258aa8098 --- /dev/null +++ b/.changeset/artifact-packages-collection-reads.md @@ -0,0 +1,55 @@ +--- +"@objectstack/core": patch +"@objectstack/runtime": patch +--- + +fix(runtime): a multi-package artifact's collections are read from `packages[]`, not only from the flattened top level + +A release artifact composed with `manifest: 'preserve'` carries every +definition twice — flattened at its top level, and again under +`packages[]` (ADR-0130 D4). Only two readers had ever learned the second +half: `ObjectQLPlugin`'s manifest service and the metadata artifact door. +Every other reader said `artifact.` and nothing else, so an +artifact that carried a collection under `packages[]` alone reached them +EMPTY — and nothing threw. The app booted clean having lost its +declarative actions, its scheduled jobs, its seed data, its object routing +or its default permission set. + +`resolveArtifactCollections` (`@objectstack/core`) is now the one way to +read a top-level collection out of an artifact in either shape. It takes +the artifact's own top-level value first and whole, then adds from each +package body — in `resolveArtifactPackageOrder`'s dependency order — the +items the top level did not already claim. A bundle that carries no +`packages[]` is returned unchanged, by identity: every single-package +artifact and every `defineStack()` config reads exactly as before. + +Taught to use it, in `@objectstack/runtime`: + +- `AppPlugin` — declared datasources and their auto-connect, the + `datasourceMapping` object routing, the objects handed to the connection + service and to the hot-reload seeder, scheduled jobs, seed datasets, + translation bundles, and the ADR-0057 security collections + (`positions` / `permissions` / `capabilities` / `sharingRules`). A job + handler's `ctx.bundle` is now the resolved view too, so + `ctx.bundle.objects` answers on a multi-package artifact. +- `collectBundleActions`, `collectBundleHooks` and + `collectBundleFunctionEntries` — including the object-EMBEDDED actions + that ride on `objects[]` and disappeared with it. +- `mergeRuntimeModule` — the declaration half. The sibling ESM module + re-supplies every callable regardless of shape, so `functions` was not + absent: a function declared `effect: 'writes'` simply came back as a bare + callable and defaulted to `'pure'`. It registered, it ran, and its writes + were counted as none. +- `createStandaloneStack`'s surfaced `requires` / `objects` / + `permissions` / `positions`, which drive the CLI's tier resolution, its + engine and storage-driver auto-registration, and the ADR-0056 D7 default + permission set. +- `resolve-project-database`'s project-database tier, which opens the + artifact itself and runs before any stack exists (`os dev`, `os start`, + `os db clean`). Without this a multi-package project silently fell + through to the unified default database instead of the datasource it + declared. + +Nothing about what the platform EMITS changes: `composeStacks` and the +artifact format are untouched, and the flattened top level is still +written. This is the reader half of the option-B program (#14512). From a7b7f884850cf23471cd185f5cf41fa9e4a90e0f Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 4 Sep 2026 06:49:36 +0000 Subject: [PATCH 3/7] test(runtime): type the option-B job handler double's argument MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `check:test-typecheck` reads packages/runtime/tsconfig.test.json, which includes this file — the mock's inferred `() => Promise` refused the `(jobCtx) => …` implementation `mockImplementation` supplies. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01UHvF5hyiZjnCyExFnfQB8m --- packages/runtime/src/app-plugin.option-b-packages.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/runtime/src/app-plugin.option-b-packages.test.ts b/packages/runtime/src/app-plugin.option-b-packages.test.ts index 356d1a34de..6d63be8f61 100644 --- a/packages/runtime/src/app-plugin.option-b-packages.test.ts +++ b/packages/runtime/src/app-plugin.option-b-packages.test.ts @@ -171,7 +171,7 @@ describe('#15005 — AppPlugin schedules and hands out `packages[]` collections' const fireReady = async () => { for (const cb of readyHooks) await cb(); }; it('schedules a job declared in a package body, and its handler context carries the RESOLVED bundle', async () => { - const sweep = vi.fn(async () => undefined); + const sweep = vi.fn(async (_jobCtx: any) => undefined as unknown); const plugin = new AppPlugin(optionBBundle({ packages: [ { manifest: { ...ordersBody(), functions: { obSweep: sweep } } }, From 306aed44303da8cbd0088bbb6bafc1e22f2d413c Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 4 Sep 2026 08:52:33 +0000 Subject: [PATCH 4/7] refactor(runtime): keep the option-B collection reader package-private, and refuse mixed spellings Contract review of PR #15261 rejected publishing this resolution from `@objectstack/core`: the "three consumers in packages that cannot import each other" premise is false (`cli -> runtime`, `cli -> plugin-security` and `runtime -> plugin-security` all exist today; only `plugin-security -> runtime` would cycle), and the two sibling reader cards landed their own private `packages[]` walks, so there is exactly one consumer. Maintainer decision 2026-09-04: those two land as they are and this card publishes nothing. - Move `artifact-collections.ts` and its tests to `@objectstack/runtime`, where every call site already lives. `packages/core/src/index.ts` is restored byte-for-byte, so `@objectstack/core` has no source change in this PR at all. The module is not named by `packages/runtime/src/index.ts`, so neither symbol reaches a published surface. - Drop the partly-flattened support claim and its test. #14512 ruled "Not D (a partly flattened artifact is a new permanent shape)", and on such an artifact the top level's name claims are applied to every package body, so a second package's same-named permission set or object extension is dropped. The module header now records that instead of promising the opposite. - Refuse a collection key spelled both ways rather than skipping one. `functions` is `z.union([z.record(...), z.array(...)])`, so two packages can each be schema-valid and disagree; the previous code let `contributions[0]` pick the shape and `continue`d past the rest, losing a whole package's collection in both directions with nothing thrown. New ADR-0112 envelope `MIXED_ARTIFACT_COLLECTION_SHAPE` (422), matching what `composeStacks` already does with the same mix, classified in `dispatcher-error-vocabulary.ts`. - Document the dependency-cycle throw in `@throws`: it is a bare `Error` from `resolvePluginOrder`, so a caller matching on `err.code` / `err.status` does not match it. - Changeset drops `@objectstack/core` entirely; `@objectstack/runtime` stays `patch` because no published surface widens. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01UHvF5hyiZjnCyExFnfQB8m --- .../artifact-packages-collection-reads.md | 26 ++- packages/core/src/index.ts | 8 - packages/runtime/src/app-plugin.ts | 15 +- .../src/artifact-collections.test.ts | 101 ++++++++-- .../src/artifact-collections.ts | 182 +++++++++++++++--- .../src/dispatcher-error-vocabulary.ts | 24 +++ packages/runtime/src/load-artifact-bundle.ts | 2 +- .../runtime/src/resolve-project-database.ts | 2 +- packages/runtime/src/standalone-stack.ts | 2 +- 9 files changed, 292 insertions(+), 70 deletions(-) rename packages/{core => runtime}/src/artifact-collections.test.ts (71%) rename packages/{core => runtime}/src/artifact-collections.ts (56%) diff --git a/.changeset/artifact-packages-collection-reads.md b/.changeset/artifact-packages-collection-reads.md index 0258aa8098..e306662df5 100644 --- a/.changeset/artifact-packages-collection-reads.md +++ b/.changeset/artifact-packages-collection-reads.md @@ -1,5 +1,4 @@ --- -"@objectstack/core": patch "@objectstack/runtime": patch --- @@ -15,13 +14,24 @@ EMPTY — and nothing threw. The app booted clean having lost its declarative actions, its scheduled jobs, its seed data, its object routing or its default permission set. -`resolveArtifactCollections` (`@objectstack/core`) is now the one way to -read a top-level collection out of an artifact in either shape. It takes -the artifact's own top-level value first and whole, then adds from each -package body — in `resolveArtifactPackageOrder`'s dependency order — the -items the top level did not already claim. A bundle that carries no -`packages[]` is returned unchanged, by identity: every single-package -artifact and every `defineStack()` config reads exactly as before. +`resolveArtifactCollections` — new, and PACKAGE-PRIVATE to +`@objectstack/runtime` — is now the one way this package reads a top-level +collection out of an artifact in either shape. It takes the artifact's own +top-level value first and whole, then adds from each package body — in +`resolveArtifactPackageOrder`'s dependency order — the items the top level +did not already claim. A bundle that carries no `packages[]` is returned +unchanged, by identity: every single-package artifact and every +`defineStack()` config reads exactly as before. Nothing is added to any +package's published surface: `@objectstack/core` is untouched by this +change, and the new module is not named by +`packages/runtime/src/index.ts`. + +Where one collection key is spelled two ways inside one artifact — +`functions` is `z.union([z.record(…), z.array(…)])`, so two packages can +each be schema-valid and disagree — the read is REFUSED with an ADR-0112 +envelope (`MIXED_ARTIFACT_COLLECTION_SHAPE`, 422) rather than one spelling +being skipped. `composeStacks` already refuses the same mix at compose +time for the same reason. Taught to use it, in `@objectstack/runtime`: diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index a41c2adda7..7e9d3eb3f0 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -17,14 +17,6 @@ export * from './plugin-order.js'; // because the ordering it performs is `resolvePluginOrder` directly above. // `@objectstack/objectql` re-exports it, so its published surface is unchanged. export * from './artifact-packages.js'; -// ADR-0130 D4 / option B (#15005) — the ONE way to read a top-level -// collection out of an artifact that may carry it flattened, under -// `packages[]`, or both. Beside `artifact-packages.js` and for the same -// reason: its readers live in packages that cannot import each other -// (`@objectstack/runtime`, `@objectstack/cli`, `@objectstack/plugin-security`), -// and N private `packages[]` walks would disagree about what an artifact -// CONTAINS rather than merely about the order. -export * from './artifact-collections.js'; export * from './lite-kernel.js'; export * from './types.js'; export * from './logger.js'; diff --git a/packages/runtime/src/app-plugin.ts b/packages/runtime/src/app-plugin.ts index 1cb519eddb..aa24315ee2 100644 --- a/packages/runtime/src/app-plugin.ts +++ b/packages/runtime/src/app-plugin.ts @@ -1,6 +1,7 @@ // Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. -import { Plugin, PluginContext, resolveArtifactCollections, wireAuthoredTranslationSync } from '@objectstack/core'; +import { Plugin, PluginContext, wireAuthoredTranslationSync } from '@objectstack/core'; +import { resolveArtifactCollections } from './artifact-collections.js'; import { applyArtifactForwardConversions, assertProtocolCompat } from '@objectstack/metadata-core'; import { resolveTenancyPosture } from '@objectstack/types'; import { postureEnforcesWall, type TenancyPosture } from '@objectstack/spec/security'; @@ -144,9 +145,9 @@ export class AppPlugin implements Plugin { /** * `this.bundle` with every package-owned collection resolved across BOTH * artifact shapes — the flattened top level and `packages[]` (ADR-0130 D4 / - * option B, #15005; the resolution itself is - * `resolveArtifactCollections` in `@objectstack/core`, shared with the - * other readers of the same key). + * option B, #15005; the resolution itself is `resolveArtifactCollections` + * in `./artifact-collections.ts`, package-private because every call site + * it has today ships in this package). * * ⛔ Read COLLECTIONS through this, never `this.bundle` — `this.bundle` is * what the caller handed us and, on a multi-package option-B artifact, its @@ -1882,9 +1883,9 @@ export class AppPlugin implements Plugin { // through `resolveArtifactCollections` FIRST, so a multi-package artifact that // carries the collection under `packages[]` instead of flattened at the top // level is read rather than silently seen as empty. The resolution is shared -// (`@objectstack/core`) rather than open-coded per collector: three walks of -// one `packages[]` that ordered or de-duplicated differently would disagree -// about what the artifact CONTAINS. On any bundle without `packages[]` it +// (`./artifact-collections.ts`) rather than open-coded per collector: three +// walks of one `packages[]` that ordered or de-duplicated differently would +// disagree about what the artifact CONTAINS. On any bundle without `packages[]` it // returns the argument itself, so these three functions are unchanged for every // single-package artifact and every `defineStack()` config. diff --git a/packages/core/src/artifact-collections.test.ts b/packages/runtime/src/artifact-collections.test.ts similarity index 71% rename from packages/core/src/artifact-collections.test.ts rename to packages/runtime/src/artifact-collections.test.ts index 74e2b3be6b..db6d2a7861 100644 --- a/packages/core/src/artifact-collections.test.ts +++ b/packages/runtime/src/artifact-collections.test.ts @@ -14,11 +14,17 @@ * it; * 2. an option-B artifact yields the package bodies' collections, in * `resolveArtifactPackageOrder`'s order; - * 3. a PARTIALLY flattened artifact resolves per item, which is the state a - * "top level, else `packages[]`" fallback answers wrongly; - * 4. a key NO source declares stays ABSENT rather than becoming `[]` — + * 3. a key NO source declares stays ABSENT rather than becoming `[]` — * `createStandaloneStack` omits `objects` on that basis and consumers gate - * on the key's presence. + * on the key's presence; + * 4. two sources spelling one collection differently are REFUSED with an + * ADR-0112 envelope rather than one of them being skipped. + * + * ⛔ There is deliberately NO test for a partially flattened artifact. #14512 + * ruled "⛔ Not D (a partly flattened artifact is a new permanent shape)", so + * the emitter flips whole-artifact and that state does not occur; the module + * header records what it would actually do on one, which is not something to + * pin as a guarantee. */ import { describe, it, expect } from 'vitest'; @@ -177,18 +183,6 @@ describe('resolveArtifactCollections', () => { ]); }); - it('resolves a PARTIALLY flattened artifact per item, not per artifact', () => { - // The transition state: one package's collections are flattened, the - // other's are not. "Use the top level when present, else `packages[]`" - // answers this one wrongly and silently. - const partial = { - objects: [obj('account')], - packages: packagesOf({ objects: [obj('account')] }, { objects: [obj('order')] }), - }; - const resolved = resolveArtifactCollections(partial) as Record; - expect(resolved.objects.map((o: any) => o.name)).toEqual(['account', 'order']); - }); - it('merges the RECORD spelling of a collection, top level winning', () => { // `functions` is a map, and `datasources` is legitimately either shape. const artifact = { @@ -208,6 +202,81 @@ describe('resolveArtifactCollections', () => { expect('jobs' in resolved).toBe(false); }); + it('REFUSES a collection spelled both ways, in BOTH orders — never skips one', () => { + // `functions` is `z.union([z.record(…), z.array(…)])` in + // `packages/spec/src/stack.zod.ts`, so BOTH spellings pass + // `AssembledPackageBodySchema` and two packages in one artifact can each + // be valid and disagree. Skipping the losing spelling drops a whole + // package's collection with nothing thrown — on `functions` that is a + // handler declared `effect: 'writes'` coming back bare and defaulting to + // `'pure'`, the sharpest loss the whole reader program exists to close. + // + // Both directions are driven because the defect was order-dependent: + // whichever spelling came first decided the shape and the other was + // dropped, so a single-direction test passes over half the defect. + const recordForm = { syncBilling: { handler: 'syncBilling', effect: 'writes' } }; + const arrayForm = [{ name: 'sendMail', handler: 'sendMail', effect: 'writes' }]; + + // `core` sorts FIRST (`orders` depends on it), so this is record-then-array. + let raised: any; + try { + resolveArtifactCollections({ + packages: packagesOf({ functions: recordForm }, { functions: arrayForm }), + }); + } catch (err) { + raised = err; + } + expect(raised?.code).toBe('MIXED_ARTIFACT_COLLECTION_SHAPE'); + expect(raised?.status).toBe(422); + expect(raised?.message).toContain('`functions`'); + + // …and array-then-record, which the pre-refusal code lost in the other + // direction (it dropped `syncBilling` instead of `sendMail`). + let reversed: any; + try { + resolveArtifactCollections({ + packages: packagesOf({ functions: arrayForm }, { functions: recordForm }), + }); + } catch (err) { + reversed = err; + } + expect(reversed?.code).toBe('MIXED_ARTIFACT_COLLECTION_SHAPE'); + expect(reversed?.status).toBe(422); + }); + + it('refuses a mix against the flattened TOP LEVEL too, and names both sources', () => { + let raised: any; + try { + resolveArtifactCollections({ + functions: { syncBilling: { handler: 'syncBilling', effect: 'writes' } }, + packages: packagesOf({ functions: [{ name: 'sendMail', handler: 'sendMail' }] }), + }); + } catch (err) { + raised = err; + } + expect(raised?.code).toBe('MIXED_ARTIFACT_COLLECTION_SHAPE'); + expect(raised?.status).toBe(422); + // The message has to say WHICH two sources disagree, or the author has + // no way to act on it: an artifact carries N packages and the refusal + // names one collection key. + expect(raised?.message).toContain("the artifact's flattened top level"); + expect(raised?.message).toContain('com.example.core'); + }); + + it('leaves a collection spelled ONE way alone — the refusal is not a shape check on each source', () => { + // The anti-vacuity control for the two tests above: the same record + // spelling in both bodies resolves, so the refusal discriminates a MIX + // rather than firing on `functions` at all. + const resolved = resolveArtifactCollections({ + packages: packagesOf( + { functions: { syncBilling: { handler: 'syncBilling', effect: 'writes' } } }, + { functions: { sendMail: { handler: 'sendMail' } } }, + ), + }) as Record; + expect(Object.keys(resolved.functions).sort()).toEqual(['sendMail', 'syncBilling']); + expect(resolved.functions.syncBilling.effect).toBe('writes'); + }); + it('raises the load path\'s OWN refusal for a malformed `packages[]`', () => { // ADR-0112 envelope, from `resolveArtifactPackageOrder` — the same // refusal `ObjectQLPlugin`'s `manifest` service raises on these bytes. diff --git a/packages/core/src/artifact-collections.ts b/packages/runtime/src/artifact-collections.ts similarity index 56% rename from packages/core/src/artifact-collections.ts rename to packages/runtime/src/artifact-collections.ts index 963dbc4b42..719f9af9aa 100644 --- a/packages/core/src/artifact-collections.ts +++ b/packages/runtime/src/artifact-collections.ts @@ -21,15 +21,33 @@ * #15004's acceptance pin (`packages/cli/test/option-b-reader-acceptance.pin.test.ts`) * measured 24 such subsystems. * - * ## What this module is, and why it is ONE function + * ## Why this is ONE function, and why it is PRIVATE to this package * * The enumeration missed reader sites twice (#14512 comments 5523603341 and * 5523741937). N readers each growing their own `packages[]` walk is that same * miss with a longer tail: two walks that ordered or de-duplicated differently * would disagree about what an artifact CONTAINS, not merely about the order. - * So this is the one resolution, in the package all three reader cards - * (#15005 runtime · #15006 cli · #15007 plugin-security) already depend on, and - * it sits beside `resolveArtifactPackageOrder` because it is built out of it. + * So `@objectstack/runtime`'s ~12 reader sites resolve through this one + * function, and it sits on top of `resolveArtifactPackageOrder` because it is + * built out of it. + * + * ⛔ It is NOT exported from this package, and it does not live in + * `@objectstack/core`. The argument that would put it there — "its readers live + * in packages that cannot import each other" — is not true of this repository: + * `@objectstack/cli` already depends on both `@objectstack/runtime` and + * `@objectstack/plugin-security`, and `@objectstack/runtime` already depends on + * `@objectstack/plugin-security`; only `plugin-security → runtime` would close a + * cycle. What actually decides the home is PULL, and today there is exactly one + * consumer: every call site of this function ships in this package. The two + * sibling reader cards resolved `packages[]` privately instead (#15006 in + * `packages/cli/src/utils/stack-collections.ts`, #15007 inside + * `@objectstack/plugin-security`), so publishing a shared surface here would + * publish it for nobody. Maintainer decision, 2026-09-04: those two land as they + * are, and this card publishes nothing. + * + * When a second package genuinely needs this resolution, the home question is + * decided THEN, with the second consumer in hand — and a symbol that was never + * published can move without a major. * * ⛔ Ordering is NOT re-derived here. `resolveArtifactPackageOrder` (ADR-0130 * D4/D5, the platform's one artifact package sorter, reused by @@ -56,9 +74,17 @@ * - **On an option-B artifact the result is package order**, because the top * level contributes nothing and step 2 is the whole answer. * - * A partially-flattened artifact — the real transition state, and the case a - * "use the top level, else `packages[]`" fallback would get wrong — is - * resolved per key and per item rather than per artifact. + * ⛔ A PARTIALLY flattened artifact is not a shape this claims to resolve, and + * this module deliberately carries no test asserting that it does. #14512's + * ruling is explicit — "⛔ Not D (a partly flattened artifact is a new permanent + * shape)" — so the emitter flips whole-artifact and the half-flattened state is + * ruled out rather than supported. Stating otherwise would have been worse than + * silence: on such an artifact the top level's NAME claims (below) apply to + * every package body, so a second package's same-named entry — its own + * `default_profile` permission set, its own extension of a shared object — + * reads as already claimed and is dropped. Do not add a claim, a test or a + * fallback for that shape; if the emitter is ever asked to produce one, that is + * a new ruling and this rule is re-derived under it. * * ## What "the top level already claimed it" means * @@ -81,6 +107,22 @@ * concatenate on the additive shape (`COMPOSE_KEY_DISPOSITIONS`), and they * concatenate here too, so the two shapes agree. * + * ## Mixed spellings are REFUSED, never skipped + * + * Two collection keys are legitimately writable in more than one shape: + * `functions` is `z.union([z.record(…), z.array(…)])` and `datasources` is read + * as either an array or a `name -> definition` record. Both spellings pass + * `AssembledPackageBodySchema`, so two packages in one artifact can each be + * valid and disagree. Merging them would have to invent the half the other + * spelling does not carry (an array entry names itself and may declare + * `packageId`; a record entry is named by its key), and skipping the losing + * spelling would lose a whole package's collection in silence — on `functions` + * that means a handler declared `effect: 'writes'` coming back as a bare + * callable and defaulting to `'pure'`, which is the exact loss this program + * exists to close. So the mix is REFUSED with an ADR-0112 envelope, matching + * what `composeStacks` already does at compose time (`composeFunctions`, + * `packages/spec/src/stack.zod.ts`) and for the same stated reason. + * * ## The key set is DERIVED * * `ObjectStackDefinitionSchema` ∩ `AssembledPackageBodySchema` is precisely @@ -93,10 +135,9 @@ * reads it instead of deriving it and nothing else here changes.) */ +import { artifactPackageId, resolveArtifactPackageOrder } from '@objectstack/core'; import { AssembledPackageBodySchema, ObjectStackDefinitionSchema } from '@objectstack/spec'; -import { resolveArtifactPackageOrder } from './artifact-packages.js'; - /** A Zod object schema, read for its declared key set only. */ type KeyedShape = { shape: Record }; @@ -114,6 +155,14 @@ let cachedCollectionKeys: readonly string[] | undefined; * entries `resolveArtifactPackageOrder` has already parsed against * `ArtifactPackageSchema` — which embeds `AssembledPackageBodySchema` — so the * shapes are built by the time the first call needs them. + * + * ⚠️ `export` here is MODULE scope, not package surface: this module is not + * named by `packages/runtime/src/index.ts`, so neither this function nor + * {@link resolveArtifactCollections} appears in `dist/index.d.ts` and neither is + * importable from `@objectstack/runtime`. The keyword is here because + * `artifact-collections.test.ts` imports it by module path — the derivation is + * the part of this file that fails silently if it ever stops being a + * derivation, so it is pinned directly rather than inferred from a merge. */ export function packageOwnedCollectionKeys(): readonly string[] { if (cachedCollectionKeys !== undefined) return cachedCollectionKeys; @@ -122,6 +171,21 @@ export function packageOwnedCollectionKeys(): readonly string[] { return cachedCollectionKeys; } +/** + * Refusals raised by {@link resolveArtifactCollections} itself, as ADR-0112 + * envelopes (`code` + `status`) — the shape this repository's rejection tests + * assert against, never a bare throw. The refusals it merely PROPAGATES come + * from `resolveArtifactPackageOrder` and keep that module's codes. + */ +type ArtifactCollectionError = Error & { code: string; status: number }; + +function refuse(code: string, message: string): ArtifactCollectionError { + const err = new Error(message) as ArtifactCollectionError; + err.code = code; + err.status = 422; + return err; +} + /** * A deterministic serialization of `value`, with object keys sorted so two * copies of one definition that differ only in key order still compare equal. @@ -187,6 +251,25 @@ function claimedIdentities(items: readonly unknown[]): { const isRecord = (v: unknown): v is Record => v !== null && typeof v === 'object' && !Array.isArray(v); +/** One declared value for one collection key, and where it came from. */ +interface Contribution { + /** How the refusal names this source to the author. */ + readonly label: string; + readonly value: unknown; +} + +/** The three shapes a collection value can take, as this module tells them apart. */ +type CollectionShape = 'array' | 'record' | 'other'; + +const shapeOf = (value: unknown): CollectionShape => + Array.isArray(value) ? 'array' : isRecord(value) ? 'record' : 'other'; + +const SHAPE_LABELS: Readonly> = { + array: 'the ARRAY form', + record: 'the RECORD form (`name -> entry`)', + other: 'neither the array nor the record form', +}; + /** * Merge one collection key across the artifact's top level and its package * bodies, per the module header's rule. @@ -196,24 +279,46 @@ const isRecord = (v: unknown): v is Record => * one. That distinction is read downstream: `createStandaloneStack` omits * `objects` entirely when the artifact declares none, and consumers of that * result gate on the key's presence. + * @throws `MIXED_ARTIFACT_COLLECTION_SHAPE` (ADR-0112, 422) when two sources + * declare this key in different shapes — see the module header. */ -function mergeCollection(top: unknown, fromBodies: readonly unknown[]): unknown { - const contributions = [top, ...fromBodies].filter((v) => v !== undefined && v !== null); +function mergeCollection(key: string, top: unknown, fromBodies: readonly Contribution[]): unknown { + const contributions = [{ label: "the artifact's flattened top level", value: top }, ...fromBodies] + .filter((c) => c.value !== undefined && c.value !== null); if (contributions.length === 0) return undefined; - // The first declared contribution decides the shape. `datasources` is - // legitimately either an array or a `name -> definition` record (AppPlugin - // reads both), and `functions` is a record; mixing the two spellings inside - // one artifact is not a shape this merges — the first one wins and the rest - // of that kind join it, which is what the pre-option-B reader did with the - // one copy it had. - if (Array.isArray(contributions[0])) { + // Every source must agree on the shape before anything is merged. ⛔ Not + // "the first one wins and the rest of that kind join it": that reading + // drops a whole package's collection with nothing thrown, which is the one + // outcome this program forbids. See the module header for why refusing is + // the same answer `composeStacks` gives the same mix. + const shape = shapeOf(contributions[0].value); + const divergent = contributions.find((c) => shapeOf(c.value) !== shape); + if (divergent !== undefined) { + throw refuse( + 'MIXED_ARTIFACT_COLLECTION_SHAPE', + `Release artifact collection \`${key}\` is declared in ${SHAPE_LABELS[shape]} by ` + + `${contributions[0].label} and in ${SHAPE_LABELS[shapeOf(divergent.value)]} by ` + + `${divergent.label}. Both spellings can be schema-valid — \`functions\` is declared as ` + + '`z.union([z.record(…), z.array(…)])` and `datasources` is read in either shape — so ' + + 'neither side is a mistake this can correct, and merging them would have to invent the ' + + 'half the other spelling does not carry (an array entry names itself and may declare ' + + '`packageId`; a record entry is named by its key). Taking one and skipping the other ' + + `would lose the whole of one package's \`${key}\` with nothing thrown. \`composeStacks\` ` + + 'refuses the same mix at compose time for the same reason (`composeFunctions`, ' + + '`packages/spec/src/stack.zod.ts`), so an artifact carrying it was not produced by one ' + + `\`composeStacks\` run. Fix: author \`${key}\` in the same shape in every package of one ` + + 'artifact — the record form is preferred for `functions`.', + ); + } + + if (shape === 'array') { const base = Array.isArray(top) ? top : []; const claimed = claimedIdentities(base); const out = [...base]; for (const contribution of fromBodies) { - if (!Array.isArray(contribution)) continue; - for (const item of contribution) { + if (!Array.isArray(contribution.value)) continue; + for (const item of contribution.value) { if (claimed.has(item)) continue; out.push(item); } @@ -224,20 +329,21 @@ function mergeCollection(top: unknown, fromBodies: readonly unknown[]): unknown return out.length === base.length && Array.isArray(top) ? top : out; } - if (isRecord(contributions[0])) { + if (shape === 'record') { const out: Record = {}; for (const contribution of contributions) { - if (!isRecord(contribution)) continue; - for (const [key, value] of Object.entries(contribution)) { - if (!(key in out)) out[key] = value; + for (const [entryKey, value] of Object.entries(contribution.value as Record)) { + if (!(entryKey in out)) out[entryKey] = value; } } return isRecord(top) && Object.keys(out).length === Object.keys(top).length ? top : out; } - // A scalar collection value is not a shape any collection key declares; - // hand back what the artifact carried rather than inventing a merge. - return top !== undefined && top !== null ? top : contributions[0]; + // A scalar collection value is not a shape any collection key declares, and + // the agreement check above has already established that it is the ONLY + // shape present — so there is nothing to merge it with. Hand back what the + // artifact carried rather than inventing a merge. + return top !== undefined && top !== null ? top : contributions[0].value; } /** @@ -260,19 +366,39 @@ function mergeCollection(top: unknown, fromBodies: readonly unknown[]): unknown * raises on the same artifact during boot. Resolving collections out of an * artifact the loader would refuse is not a quieter outcome, it is a * different answer to what the artifact contains. + * @throws `MIXED_ARTIFACT_COLLECTION_SHAPE` (ADR-0112, 422) — this module's own + * refusal — when one collection key is declared in the array form by one + * source and in the record form by another. + * @throws ⚠️ `resolvePluginOrder`'s dependency-CYCLE error, propagated through + * `resolveArtifactPackageOrder` when two packages inside one artifact depend + * on each other. Unlike the three refusals above it is a BARE `Error`: it + * carries no `code` and no `status`, so a caller matching on `err.code` / + * `err.status` does not match it and falls through to its generic branch. + * That is `resolveArtifactPackageOrder`'s own contract — reached identically + * by `ObjectQLPlugin`'s manifest service today — and enveloping it would + * change that function's behaviour for every caller, which is not this + * module's call to make. Recorded here so a caller writing a `catch` knows + * the third shape exists. */ export function resolveArtifactCollections(artifact: T): T { if (artifact === null || typeof artifact !== 'object') return artifact; if (!Array.isArray((artifact as { packages?: unknown }).packages)) return artifact; const bodies = resolveArtifactPackageOrder(artifact) as Array | null | undefined>; + // `resolveArtifactPackageOrder` has already refused any entry whose manifest + // carries no usable id, so every body can be named in a refusal message. + const labels = bodies.map((body) => `package "${artifactPackageId(body) ?? ''}"`); const source = artifact as unknown as Record; let resolved: Record | undefined; for (const key of packageOwnedCollectionKeys()) { const merged = mergeCollection( + key, source[key], - bodies.map((body) => (body !== null && typeof body === 'object' ? body[key] : undefined)), + bodies.map((body, index) => ({ + label: labels[index], + value: body !== null && typeof body === 'object' ? body[key] : undefined, + })), ); if (merged === source[key]) continue; if (merged === undefined) continue; diff --git a/packages/runtime/src/dispatcher-error-vocabulary.ts b/packages/runtime/src/dispatcher-error-vocabulary.ts index bd7407cc02..d93312714a 100644 --- a/packages/runtime/src/dispatcher-error-vocabulary.ts +++ b/packages/runtime/src/dispatcher-error-vocabulary.ts @@ -820,6 +820,30 @@ export const UNREGISTERED_CODE_SITES: readonly UnregisteredCodeSite[] = [ 'repo\'s rejection tests assert on, not evidence of a door. If an install door ever answers with ' + 'this code itself, the verdict becomes pending-registration and it belongs in the ledger batch.' }, + // [ADR-0130 D4 / option B] The reader half's own refusal, raised where the + // three above are merely propagated. Same pre-HTTP reasoning; what is + // specific to it is recorded in its `why`. + { + code: 'MIXED_ARTIFACT_COLLECTION_SHAPE', + file: 'packages/runtime/src/artifact-collections.ts', + shape: 'codehelper', + door: 'none', + verdict: 'boot-refusal', + why: + 'The refusal for one collection key declared in the ARRAY form by one source and in the ' + + 'RECORD form by another inside the same artifact — `functions` is ' + + '`z.union([z.record(…), z.array(…)])` and `datasources` is read in either shape, so both ' + + 'sides can pass `AssembledPackageBodySchema` and still disagree. Raised by ' + + '`resolveArtifactCollections`, whose every call site ships in this package and runs BEFORE ' + + 'any HTTP boundary exists: `app-plugin.ts` resolves inside plugin init (a throw aborts boot), ' + + '`load-artifact-bundle.ts` and `standalone-stack.ts` resolve while the artifact is being ' + + 'loaded into a stack that has no transport yet, and `resolve-project-database.ts` opens the ' + + 'artifact file to pick a database before a kernel exists at all. The function is not exported ' + + 'from `packages/runtime/src/index.ts`, so no package outside this one can reach it to put the ' + + 'code on a wire. Its `status: 422` is the ADR-0112 envelope shape this repo\'s rejection ' + + 'tests assert on, not evidence of a door. If an install or serve door ever answers with this ' + + 'code itself, the verdict becomes pending-registration and it belongs in the ledger batch.' + }, { code: 'DUPLICATE_ARTIFACT_OBJECT_NAME', file: 'packages/objectql/src/registry.ts', diff --git a/packages/runtime/src/load-artifact-bundle.ts b/packages/runtime/src/load-artifact-bundle.ts index f0d471be18..3aff1a94bf 100644 --- a/packages/runtime/src/load-artifact-bundle.ts +++ b/packages/runtime/src/load-artifact-bundle.ts @@ -30,7 +30,7 @@ import { readFile } from 'node:fs/promises'; import { resolve as resolvePath, isAbsolute, dirname } from 'node:path'; import { pathToFileURL } from 'node:url'; -import { resolveArtifactCollections } from '@objectstack/core'; +import { resolveArtifactCollections } from './artifact-collections.js'; export interface LoadArtifactBundleOptions { /** Optional log tag for warnings (defaults to `[loadArtifactBundle]`). */ diff --git a/packages/runtime/src/resolve-project-database.ts b/packages/runtime/src/resolve-project-database.ts index 6114c33d26..894a8bb701 100644 --- a/packages/runtime/src/resolve-project-database.ts +++ b/packages/runtime/src/resolve-project-database.ts @@ -44,7 +44,7 @@ import { resolve as resolvePath, isAbsolute } from 'node:path'; import { existsSync, readFileSync } from 'node:fs'; import { homedir } from 'node:os'; -import { resolveArtifactCollections } from '@objectstack/core'; +import { resolveArtifactCollections } from './artifact-collections.js'; import { resolveDatabaseDriverId, resolveDriverId } from '@objectstack/spec/data'; /** The unified default database filename (`/data/objectstack.db`). */ diff --git a/packages/runtime/src/standalone-stack.ts b/packages/runtime/src/standalone-stack.ts index 31249428d2..ce4f68875e 100644 --- a/packages/runtime/src/standalone-stack.ts +++ b/packages/runtime/src/standalone-stack.ts @@ -57,7 +57,7 @@ import { mkdirSync, existsSync } from 'node:fs'; import { homedir } from 'node:os'; import { z } from 'zod'; import { stampSearchPinyinEnabled } from '@objectstack/types'; -import { resolveArtifactCollections } from '@objectstack/core'; +import { resolveArtifactCollections } from './artifact-collections.js'; import { BUILTIN_DRIVER_IDS, DATABASE_DRIVER_SELECTION_ALIASES, From ccdc22b8e256becce9750c07a4d82e165f9598dd Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 4 Sep 2026 11:14:45 +0000 Subject: [PATCH 5/7] test(cli): record the option-B ledger reaching ZERO, and re-verify it there MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `OPTION_B_LOSSES` had 23 rows on `origin/main` after #15007 landed (it deleted the one row no artifact-side change could reach). All 23 belong to this card, so merging it empties the ledger — which is the state the pin's own header calls "the program is done". An empty ledger is also the state that could go vacuous, so it is re-verified rather than asserted: with `resolveArtifactCollections` neutered to the identity function and `@objectstack/runtime` REBUILT — the pin reaches that package through its `exports` map, so `dist/` is what it measures — the pin goes red naming exactly 23 rows, byte-for-byte the set the ledger carried before. Restored and rebuilt, 7 passed. ⛔ The set-equality assertion, the subsystem coverage and the anti-vacuity controls are untouched; the only edits are the ledger's own rows and the two docblocks that described them. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01UHvF5hyiZjnCyExFnfQB8m --- .../option-b-reader-acceptance.pin.test.ts | 51 ++++++++++++------- 1 file changed, 34 insertions(+), 17 deletions(-) diff --git a/packages/cli/test/option-b-reader-acceptance.pin.test.ts b/packages/cli/test/option-b-reader-acceptance.pin.test.ts index b8b7d84784..c2ae1342db 100644 --- a/packages/cli/test/option-b-reader-acceptance.pin.test.ts +++ b/packages/cli/test/option-b-reader-acceptance.pin.test.ts @@ -66,10 +66,18 @@ * * Run with `OPTION_B_LOSSES` emptied on `origin/main` `33681eaef`, the pin * reports **24 subsystems** losing their collections, across all three packages - * the program scopes — the full red output is recorded in this card's PR body. + * the program scopes — the full red output is recorded in #15004's PR body. * In the SAME run the additive baseline and the `packages[]` control both pass, * which is what makes the red a discrimination rather than a broken fixture. * + * Re-verified at the empty ledger, which is the state that could go vacuous: + * with `@objectstack/runtime`'s `resolveArtifactCollections` neutered to the + * identity function and that package REBUILT (the pin reaches it through its + * `exports` map, so `dist/` is what it measures), this pin goes RED naming + * exactly 23 rows — the same 23, byte for byte, that the ledger carried before + * #15005. Restored and rebuilt, 7 passed. So the empty ledger is a measurement + * of the readers, not of a probe that stopped looking. + * * One row in that output is worth naming here, because it is a loss no * presence-check would have found: on the compiled path a function declared * `effect: 'writes'` comes back through `collectBundleFunctionEntries` as @@ -129,25 +137,34 @@ import { measureShape, type ProbeRow, type ShapeMeasurement } from './fixtures/o /** * The subsystems that silently lose their collection when the flattened top - * level is gone — MEASURED, not curated. Opened at 24 rows on `origin/main` - * `33681eaef` (#15004); 23 of them were deleted by #15005 when - * `@objectstack/runtime` learned to resolve `packages[]` — the whole of - * boundaries B1 and B5, plus every B2 row whose reader ships in that package. + * level is gone — MEASURED, not curated. ⭐ EMPTY: the reader half of the + * option-B program is done, and the emitter half (#14512) is unblocked. + * + * How it got here, in the order the rows actually went: * - * The one row left is NOT a runtime reader: `appSecurityPluginOptions` - * (`@objectstack/plugin-security`) reads `config.permissions` off the - * from-source config directly, so nothing on the artifact side can reach it. - * Card #15007 owns it, and the ledger is empty — the reader half done, the - * emitter half #14512 unblocked — when it goes. + * - opened at **24 rows** on `origin/main` `33681eaef` (#15004); + * - **23 → 23** when #15007 (`@objectstack/plugin-security`) landed, deleting + * `B2 · plugin-security appSecurityPluginOptions over the from-source + * config`, the one row no artifact-side change could reach; + * - **23 → 0** here (#15005), when `@objectstack/runtime` learned to resolve + * `packages[]`: the whole of boundaries B1 and B5, and every B2 row whose + * reader ships in that package. * - * Its B1 twin is already gone: that row runs `appSecurityPluginOptions` over - * `createStandaloneStack`'s RESULT, and the result now surfaces `permissions` - * resolved across both shapes. Same reader, two boundaries, one of them fed by - * a producer this card fixed — which is the two-boundary split #15005 warns - * about, seen from the ledger's side. + * ⚠️ The last row to go was a plugin-security one and it is NOT #15007's twin + * arriving late: `B1 · plugin-security appSecurityPluginOptions over the + * artifact-serve config` runs that same reader over `createStandaloneStack`'s + * RESULT. Nothing inside `@objectstack/plugin-security` could move it — the + * standalone result carried neither the permission sets nor a route to them — + * and it goes green because that result now surfaces `permissions` resolved + * across both shapes, which plugin-security's existing top-level branch then + * answers. One reader, two boundaries, each owned by a different card: that + * split is what the header's B1/B2 distinction is for. * - * ⛔ SHRINK-ONLY, audited in BOTH directions (see the header). Each line names - * a boundary, a subsystem and the collection it reads. + * ⛔ SHRINK-ONLY, audited in BOTH directions (see the header). An empty ledger + * is the STRONGEST state this pin has, not a disabled one: every row the probe + * measures must now be `present` in BOTH shapes, so a reader that regresses — + * or a new reader that arrives unresolved — is red on arrival with nothing left + * to absorb it. Adding a line is never how that red is fixed. */ const OPTION_B_LOSSES: readonly string[] = []; From 38d3f248b4532788092d176b4c39d389c7a72dd6 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 4 Sep 2026 11:20:03 +0000 Subject: [PATCH 6/7] test(runtime): pin `packages: []` returning by identity, which no test covered MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The three identity controls the design rests on are the additive artifact returning by reference, `packages: []` returning unchanged, and an artifact with no `packages` key short-circuiting. The first and third were pinned; the second was not, and it is the one that is not obvious: `[]` IS an array, so such an artifact walks the whole resolution — package order over zero entries, then every package-owned key merged against no contributions — and still has to come back as the SAME object. Any key that came back a fresh copy would trip `{ ...artifact }` and hand every reader downstream a different object than it was given. Asserted with and without collections present, so the identity is not an artifact of the one key that happened to be there. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01UHvF5hyiZjnCyExFnfQB8m --- .../runtime/src/artifact-collections.test.ts | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/packages/runtime/src/artifact-collections.test.ts b/packages/runtime/src/artifact-collections.test.ts index db6d2a7861..b33f5e3b84 100644 --- a/packages/runtime/src/artifact-collections.test.ts +++ b/packages/runtime/src/artifact-collections.test.ts @@ -93,6 +93,25 @@ describe('resolveArtifactCollections', () => { expect(resolveArtifactCollections(odd)).toBe(odd); }); + it('returns the ARGUMENT ITSELF for an EMPTY `packages: []` too', () => { + // The second of the three identity controls, and the one the branch + // above does NOT cover: `[]` IS an array, so this artifact walks the + // whole resolution — `resolveArtifactPackageOrder` over zero entries, + // then every package-owned key merged against no contributions — and + // still has to come back as the same object. If any key came back a + // fresh copy, `{ ...artifact }` would fire and every reader downstream + // would be handed a different object than the one it was given. + const objects = [obj('account')]; + const empty = { manifest: { id: 'a', name: 'A' }, objects, packages: [] as unknown[] }; + const resolved = resolveArtifactCollections(empty); + expect(resolved).toBe(empty); + expect(resolved.objects).toBe(objects); + // …and with no collections at all, so the identity is not an artifact of + // the one key that happened to be present. + const bare = { packages: [] as unknown[] }; + expect(resolveArtifactCollections(bare)).toBe(bare); + }); + it('leaves TODAY\'s additive artifact untouched — same arrays, same order, same references', () => { const coreObject = obj('account'); const ordersObject = obj('order'); From c953ac0f82df353f9efbeb55cd994f9d9ccd1e9c Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 4 Sep 2026 12:03:17 +0000 Subject: [PATCH 7/7] test(cli): re-anchor the BASELINE anti-vacuity floor, which the empty ledger made vacuous MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `expect(additive.rows.length).toBeGreaterThanOrEqual(OPTION_B_LOSSES.length)` was a real bound only while the ledger was non-empty. This PR emptied it, so the line became `rows.length >= 0` — true of every array, including an empty one. It was dead code wearing a control's comment, and it took with it the fourth direction this file's header claims: "the probe itself quietly measuring less ⇒ RED". Re-anchored to the probe's MEASURED row count rather than deleted, because none of the three controls that survive an empty ledger covers this one: the `registryObjectsFromArtifact` CONTROL asserts two object names, and the two coverage tests assert that the five boundaries and #15006's four sites are represented — none of them notices rows disappearing. 30 is measured, not remembered: with the line temporarily written `toBe(-1)` the run reports `expected 30 to be -1`. Verified live at the boundary in the same session — a floor of 31 goes RED on the same fixture with the new message, so the assertion is not satisfied by construction. Both legs proved on disk by hash and restored to a byte-identical file. `>=` rather than `toBe` keeps the shrink-only direction the ledger uses: a row added to the probe stays green, a row that stops being measured is red. ⛔ The set-equality assertion, the `registryObjectsFromArtifact` control, the five-boundary coverage test and the #15006 four-site coverage test are untouched, and the ledger is not touched to serve this line. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01UHvF5hyiZjnCyExFnfQB8m --- .../option-b-reader-acceptance.pin.test.ts | 29 +++++++++++++++++-- 1 file changed, 27 insertions(+), 2 deletions(-) diff --git a/packages/cli/test/option-b-reader-acceptance.pin.test.ts b/packages/cli/test/option-b-reader-acceptance.pin.test.ts index c2ae1342db..6969d5f98b 100644 --- a/packages/cli/test/option-b-reader-acceptance.pin.test.ts +++ b/packages/cli/test/option-b-reader-acceptance.pin.test.ts @@ -238,8 +238,33 @@ describe('#15004 — option-B acceptance pin: every subsystem must see its colle `platform emits TODAY. This is never an option-B finding — it means the fixture stopped ` + `carrying a collection, or a reader regressed on the additive path.\n${render(additive.rows)}`, ).toEqual([]); - // Anti-vacuity: a probe that measured no rows would satisfy the line above. - expect(additive.rows.length).toBeGreaterThanOrEqual(OPTION_B_LOSSES.length); + // Anti-vacuity: the assertion above is satisfied by an EMPTY row set, so a + // probe that quietly stopped measuring has to be caught right here. + // + // ⚠️ RE-ANCHORED when the ledger reached zero. The floor used to be + // `OPTION_B_LOSSES.length`, which was a real bound only while the ledger + // was non-empty; at zero it reads `rows.length >= 0` — true of every + // array, including an empty one. It was dead code wearing a control's + // comment, and the fourth direction this file's header claims ("the probe + // itself quietly measuring less ⇒ RED") had silently stopped existing. + // + // 30 is MEASURED, not remembered: with this line temporarily written + // `expect(additive.rows.length).toBe(-1)`, the run reports + // `expected 30 to be -1`. Verified live at the boundary in the same + // session — a floor of 31 goes RED on the same fixture, so the assertion + // is not satisfied by construction. + // + // `>=` rather than `toBe` on purpose, and it is the same shrink-only + // direction the ledger uses: a row ADDED to the probe is welcome and stays + // green, a row that stops being measured is red. Raise the floor when the + // probe grows; ⛔ never lower it to make a red run green. + expect( + additive.rows.length, + `The probe measured ${additive.rows.length} rows, fewer than the 30 it measured when ` + + `this floor was set. A row that stops being measured stops being able to fail, which ` + + `is the one direction this pin cannot detect anywhere else — fix the probe rather ` + + `than the floor.`, + ).toBeGreaterThanOrEqual(30); }); // ── The pin ──────────────────────────────────────────────────────────────