diff --git a/.changeset/artifact-packages-collection-reads.md b/.changeset/artifact-packages-collection-reads.md new file mode 100644 index 0000000000..e306662df5 --- /dev/null +++ b/.changeset/artifact-packages-collection-reads.md @@ -0,0 +1,65 @@ +--- +"@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` — 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`: + +- `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). 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 aeac03e618..6969d5f98b 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,36 +137,36 @@ 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. ⭐ 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: + * + * - 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. * - * ⛔ SHRINK-ONLY, audited in BOTH directions (see the header). Each line names - * a boundary, a subsystem and the collection it reads. + * ⚠️ 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). 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[] = [ - '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 · 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 OPTION_B_LOSSES: readonly string[] = []; const render = (rows: ProbeRow[], only?: (r: ProbeRow) => boolean): string => rows @@ -230,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 ────────────────────────────────────────────────────────────── 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..6d63be8f61 --- /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 (_jobCtx: any) => undefined as unknown); + 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..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, 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'; @@ -113,6 +114,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 +142,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 `./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 + * `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 +565,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 +589,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 +607,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 +649,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 +676,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 +790,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 +1002,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 +1062,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 +1147,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 +1595,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 +1738,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 +1878,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 +// (`./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. -/** 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 +1905,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 +1947,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 +1994,9 @@ export function collectBundleFunctionEntries(bundle: any): 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('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'); + 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('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('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. + // 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/runtime/src/artifact-collections.ts b/packages/runtime/src/artifact-collections.ts new file mode 100644 index 0000000000..719f9af9aa --- /dev/null +++ b/packages/runtime/src/artifact-collections.ts @@ -0,0 +1,410 @@ +// 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. + * + * ## 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 `@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 + * `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 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 + * + * 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. + * + * ## 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 + * "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 { artifactPackageId, resolveArtifactPackageOrder } from '@objectstack/core'; +import { AssembledPackageBodySchema, ObjectStackDefinitionSchema } from '@objectstack/spec'; + +/** 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` 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; + const owned = new Set(shapeKeys(AssembledPackageBodySchema)); + cachedCollectionKeys = Object.freeze(shapeKeys(ObjectStackDefinitionSchema).filter((k) => owned.has(k))); + 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. + * + * 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); + +/** 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. + * + * @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. + * @throws `MIXED_ARTIFACT_COLLECTION_SHAPE` (ADR-0112, 422) when two sources + * declare this key in different shapes — see the module header. + */ +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; + + // 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.value)) continue; + for (const item of contribution.value) { + 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 (shape === 'record') { + const out: Record = {}; + for (const contribution of contributions) { + 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, 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; +} + +/** + * 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. + * @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, index) => ({ + label: labels[index], + value: 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/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 23e13c34f5..3aff1a94bf 100644 --- a/packages/runtime/src/load-artifact-bundle.ts +++ b/packages/runtime/src/load-artifact-bundle.ts @@ -30,6 +30,8 @@ import { readFile } from 'node:fs/promises'; import { resolve as resolvePath, isAbsolute, dirname } from 'node:path'; import { pathToFileURL } from 'node:url'; +import { resolveArtifactCollections } from './artifact-collections.js'; + export interface LoadArtifactBundleOptions { /** Optional log tag for warnings (defaults to `[loadArtifactBundle]`). */ tag?: string; @@ -138,6 +140,19 @@ export async function mergeRuntimeModule(bundle: any, artifactAbsPath: string, t console.warn(`${tag} runtime module '${moduleAbsPath}' exported no \`functions\` map`); return; } + // [ADR-0130 D4 / option B, #15005] The DECLARATION half is read + // through the resolved stack, because on a multi-package artifact it + // lives under `packages[i].manifest.functions` and the top level may + // carry nothing at all. This is the sharpest of the option-B losses and + // the only one no presence-check finds: the module below re-supplies + // every CALLABLE regardless of shape, so `functions` is not ABSENT — a + // function declared `effect: 'writes'` simply comes back as a BARE + // callable, which `normalizeFlowFunctionEntry` then defaults to + // `'pure'`. The function still registers, still runs, and its writes + // are still counted as none (#4396's silent un-declaring, third + // spelling; #4354's broken-sweep filter stays quiet on the one run that + // needed it). Identical reference on any bundle without `packages[]`. + const declaredFunctions = (resolveArtifactCollections(bundle) as { functions?: unknown })?.functions; // The ARRAY form (`[{ name, handler: '', 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..894a8bb701 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 './artifact-collections.js'; 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 a1645f87c3..4d32f1546b 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 './artifact-collections.js'; 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;