diff --git a/.changeset/artifact-load-topological.md b/.changeset/artifact-load-topological.md new file mode 100644 index 0000000000..3a8b166409 --- /dev/null +++ b/.changeset/artifact-load-topological.md @@ -0,0 +1,9 @@ +--- +'@objectstack/objectql': minor +--- + +Load a release artifact's packages in dependency-topological order (ADR-0130 D5). + +The `manifest` service now reads both artifact shapes ADR-0130 D4 declares — `packages: [...]` when present, and the singular `manifest` treated as a one-element list when absent — and registers the packages inside one artifact in dependency-topological order, resolved by `resolvePluginOrder`, the platform's single topological sorter (ADR-0116). A package that extends another package's object therefore registers after the package it extends, whatever slot the artifact's array put it in. + +An existing single-`manifest` artifact takes the second branch, by reference and unrewritten, and its registration state is unchanged (ADR-0130 D7). New: `resolveArtifactPackageOrder` is exported so every other door that grows an artifact-loading seam reads both shapes and orders them the same way. diff --git a/packages/objectql/src/artifact-load-path.test.ts b/packages/objectql/src/artifact-load-path.test.ts new file mode 100644 index 0000000000..6fbe10bd80 --- /dev/null +++ b/packages/objectql/src/artifact-load-path.test.ts @@ -0,0 +1,412 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * ADR-0130 D5 + D7 — the artifact load path registers the packages inside one + * artifact in dependency-topological order, and an existing single-`manifest` + * artifact still registers bit-identically through it. + * + * ## What the D5 pin here asserts, and why it is shaped this way + * + * ADR-0130 D5 asks for a BEHAVIOURAL pin — an artifact whose `packages` array is + * deliberately ordered extension-before-base installs, and the extension is + * verified present and in effect on the extended object — and forbids the pin + * that only asserts the sorter returned a permutation, because that one stays + * green on an implementation that computes the order and then never uses it. + * + * ⚠️ Measured while writing this pin, and recorded here because the next reader + * needs it: on today's registry the "extension in effect" half does NOT by + * itself discriminate the two implementations. `objectExtensions` register as + * CONTRIBUTORS keyed by the target FQN (`SchemaRegistry.registerObject`, + * `ownership: 'extend'`) and are folded at READ time in priority order + * (`resolveObject` → `foldExtenders`), so the fold does not care which + * contributor arrived first. Registering both orders and deep-diffing the whole + * resulting registry state — merged objects, every contributor, every item + * collection, namespace owners — produced exactly ONE difference: the order of + * the package records themselves. So: + * + * - the extension-in-effect assertion is kept, because it is D5's literal + * acceptance criterion and the property that must never regress; and + * - the assertion that DISCRIMINATES is on the registry's own installed-package + * sequence — real post-install registry state (the first item in D7's own + * comparison list), not the sorter's return value. An implementation that + * iterates `packages[]` directly writes those records in array order and this + * goes red; one that computes the order and never uses it does exactly that, + * which is the silent failure D5 exists to catch. + * + * ⛔ Do not "simplify" this file by asserting `resolvePluginOrder(...)` returned + * `[base, extender]`. That is the assertion D5 rules out by name. + */ + +import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import { ObjectKernel } from '@objectstack/core'; +import { ObjectQLPlugin } from './plugin.js'; +import { resolveArtifactPackageOrder } from './artifact-packages.js'; +import type { ObjectQL } from './engine.js'; +import type { IMetadataService } from '@objectstack/spec/contracts'; + +type ManifestService = { register(m: unknown): void | Promise }; + +/** + * Slot lookups are typed, never erased to `any`: the slot already returns its + * contract, and this repo's `slot-lookup/no-any-assignment` rule exists because + * every erasure found so far was hiding a real gap. `objectql` resolves to the + * engine class here (this test lives inside that package, so the class type is + * the local contract and carries the `registry` getter the pins read). + */ +const engineOf = (kernel: ObjectKernel): ObjectQL => kernel.getService('objectql'); + +/** One resolved object body, as far as these pins read it. */ +type ResolvedObject = { + fields?: Record; + _packageId?: string; +}; + +/** One package body, as far as these pins read it. */ +type PackageBody = { + id?: string; + objects?: Array<{ name?: string }>; + defaultDatasource?: string; + scope?: string; +}; + +/** + * The extended package: owns `crm_account`. + * + * Object bodies live INLINE on the package payload, which is what the load path + * actually receives — `AppPlugin` flattens an artifact into + * `{ ...bundle.manifest, ...bundle }` before calling `manifest.register()`, and + * `ObjectQL.registerApp` reads `manifest.objects` as definitions. + */ +const basePackage = () => ({ + id: 'com.acme.crm', + name: 'acme_crm', + version: '1.0.0', + type: 'app', + namespace: 'crm', + objects: [ + { + name: 'crm_account', + label: 'Account', + fields: { + name: { name: 'name', label: 'Name', type: 'text' }, + }, + }, + ], +}); + +/** + * The extending package: adds a field to `crm_account`, which it does not own, + * and declares that dependency the way ADR-0116 requires — in `dependencies`. + * + * Distinct namespaces on purpose: co-owning packages SHARING one namespace is + * ADR-0130 D1/D3's install-gate relaxation, a separate change. This pin is about + * ordering only and must not silently depend on that one having landed. + */ +const extenderPackage = () => ({ + id: 'com.acme.crm.cpq', + name: 'acme_cpq', + version: '1.0.0', + type: 'module', + namespace: 'cpq', + dependencies: { 'com.acme.crm': '1.0.0' }, + objectExtensions: [ + { + extend: 'crm_account', + fields: { + margin: { name: 'margin', label: 'Gross Margin', type: 'number' }, + }, + }, + ], +}); + +/** An artifact whose `packages` array is deliberately extension-BEFORE-base. */ +const extenderFirstArtifact = () => ({ + packages: [{ manifest: extenderPackage() }, { manifest: basePackage() }], +}); + +/** + * The artifact's OWN package records, in the order the registry stores them. + * + * Narrowed to the `com.acme.*` fixtures on purpose: a booted kernel installs its + * own platform packages first (`com.objectstack.metadata-objects`), and an + * assertion that included those would be pinning the kernel's boot composition + * instead of this artifact's registration order. + */ +const artifactPackageIds = (ql: ObjectQL): string[] => + ql.registry + .getAllPackages() + .map((p) => p.manifest?.id) + .filter((id: string) => typeof id === 'string' && id.startsWith('com.acme.')); + +describe('ADR-0130 D5 — the load path registers artifact packages topologically', () => { + let kernel: ObjectKernel; + + beforeEach(() => { + kernel = new ObjectKernel({ logger: { level: 'silent' }, gracefulShutdown: false }); + }); + + afterEach(async () => { + if (kernel.getState() === 'running') await kernel.shutdown(); + }); + + it('installs an extension-before-base artifact, and the extension is in effect', async () => { + await kernel.use(new ObjectQLPlugin()); + await kernel.bootstrap(); + + const manifest = kernel.getService('manifest') as ManifestService; + await manifest.register(extenderFirstArtifact()); + + const ql = engineOf(kernel); + + // (1) D5's literal acceptance criterion: the artifact installed, and the + // extension is present and in effect ON THE EXTENDED OBJECT — read the + // way every consumer reads it, through the resolved object. + const account = ql.registry.resolveObject('crm_account') as ResolvedObject | undefined; + expect(account).toBeDefined(); + expect(account?.fields?.margin).toBeDefined(); + expect(account?.fields?.margin?.type).toBe('number'); + expect(account?.fields?.margin?.label).toBe('Gross Margin'); + // The extended object still belongs to the package that owns it — an + // extension contributes, it does not take ownership. + expect(ql.registry.getObjectOwner('crm_account')?.packageId).toBe('com.acme.crm'); + + // (2) The discriminating half: the load path REGISTERED in dependency order, + // not in array order. This is post-install registry state (D7's own + // first comparison item), not the sorter's return value. + expect(artifactPackageIds(ql)).toEqual(['com.acme.crm', 'com.acme.crm.cpq']); + }); + + it('orders a three-package chain declared backwards in the array', async () => { + await kernel.use(new ObjectQLPlugin()); + await kernel.bootstrap(); + + const pkg = (id: string, deps?: Record) => ({ + id, name: id.replace(/\./g, '_'), version: '1.0.0', type: 'module', + ...(deps ? { dependencies: deps } : {}), + }); + + const manifest = kernel.getService('manifest') as ManifestService; + await manifest.register({ + packages: [ + { manifest: pkg('com.acme.c', { 'com.acme.b': '1.0.0' }) }, + { manifest: pkg('com.acme.b', { 'com.acme.a': '1.0.0' }) }, + { manifest: pkg('com.acme.a') }, + ], + }); + + expect(artifactPackageIds(engineOf(kernel))).toEqual(['com.acme.a', 'com.acme.b', 'com.acme.c']); + }); + + it('the extension reaches the metadata service too, on the extender-first artifact', async () => { + await kernel.use(new ObjectQLPlugin()); + await kernel.bootstrap(); + + const manifest = kernel.getService('manifest') as ManifestService; + await manifest.register(extenderFirstArtifact()); + + // The bridge is what Studio / AI `describe_object` / `metadata.listObjects` + // read. An artifact that installs but whose extension never reaches this + // service is the same silent failure one layer out. + const metadata = kernel.getService('metadata'); + const bridged = await metadata.getObject('crm_account') as ResolvedObject | undefined; + expect(bridged).toBeDefined(); + expect(bridged?.fields?.margin?.type).toBe('number'); + expect(bridged?._packageId).toBe('com.acme.crm'); + }); +}); + +describe('ADR-0130 D5 — the ordering behaviours are INHERITED from resolvePluginOrder', () => { + // ⛔ Neither of these is re-adjudicated by the load path: they are + // `resolvePluginOrder`'s own contract (ADR-0116), asserted here so a future + // "quick local sort" cannot pass this suite. + + it('throws on a dependency cycle between two packages in one artifact', () => { + expect(() => + resolveArtifactPackageOrder({ + packages: [ + { manifest: { id: 'com.acme.a', name: 'a', version: '1.0.0', type: 'module', dependencies: { 'com.acme.b': '1.0.0' } } }, + { manifest: { id: 'com.acme.b', name: 'b', version: '1.0.0', type: 'module', dependencies: { 'com.acme.a': '1.0.0' } } }, + ], + }), + ).toThrow(/Circular dependency/); + }); + + it('leaves a dependency on a package OUTSIDE the artifact to the installer', () => { + // `manifest.dependencies` is a map of package ids to version ranges and its + // own schema example is an external package (`@steedos/plugin-auth`). The + // artifact is not the resolution scope for those, so an id that names no + // sibling here is not an edge here — it is skipped, exactly as + // `resolvePluginOrder` skips an absent optional dependency. Reading it as a + // hard miss instead would refuse every artifact that depends on anything + // outside itself, which D7 forbids. + const ordered = resolveArtifactPackageOrder({ + packages: [ + { manifest: { id: 'com.acme.a', name: 'a', version: '1.0.0', type: 'module', dependencies: { '@steedos/plugin-auth': '^2.0.0' } } }, + ], + }) as PackageBody[]; + expect(ordered.map((m) => m.id)).toEqual(['com.acme.a']); + }); + + it('preserves declared order for packages with no edges between them', () => { + const ordered = resolveArtifactPackageOrder({ + packages: [ + { manifest: { id: 'com.acme.z', name: 'z', version: '1.0.0', type: 'module' } }, + { manifest: { id: 'com.acme.a', name: 'a', version: '1.0.0', type: 'module' } }, + ], + }) as PackageBody[]; + expect(ordered.map((m) => m.id)).toEqual(['com.acme.z', 'com.acme.a']); + }); +}); + +describe('ADR-0130 D4 — the entry WRAPPER is refused from its one declaration', () => { + // Rejection assertions carry the ADR-0112 envelope — `code` AND `status` — + // never a bare "it throws": a bare throw assertion stays green on an + // unrelated `Error` from somewhere else in the path. + + it('refuses an inlined manifest body written straight onto the array element', () => { + let caught: (Error & { code?: string; status?: number }) | undefined; + try { + resolveArtifactPackageOrder({ + packages: [{ id: 'com.acme.a', name: 'a', version: '1.0.0', type: 'module' }], + }); + } catch (e) { caught = e as Error & { code?: string; status?: number }; } + expect(caught).toBeDefined(); + expect(caught?.code).toBe('INVALID_ARTIFACT_PACKAGE_ENTRY'); + expect(caught?.status).toBe(422); + expect(caught?.message).toContain('packages[0]'); + }); + + it('refuses the same package id twice rather than silently dropping one', () => { + let caught: (Error & { code?: string; status?: number }) | undefined; + try { + resolveArtifactPackageOrder({ + packages: [ + { manifest: { id: 'com.acme.a', name: 'a', version: '1.0.0', type: 'module' } }, + { manifest: { id: 'com.acme.a', name: 'a-again', version: '1.0.0', type: 'module' } }, + ], + }); + } catch (e) { caught = e as Error & { code?: string; status?: number }; } + expect(caught).toBeDefined(); + expect(caught?.code).toBe('DUPLICATE_ARTIFACT_PACKAGE'); + expect(caught?.status).toBe(422); + }); + + it('accepts an assembled package body whose `objects` are definitions, not globs', () => { + // The load path receives assembled payloads: `ManifestSchema.objects` is + // `z.array(z.string())` (glob patterns), so a FULL body parse would refuse + // exactly what this path exists to register. The wrapper is judged; the body + // is the authoring door's job. + const ordered = resolveArtifactPackageOrder({ + packages: [{ manifest: basePackage() }], + }) as PackageBody[]; + expect(ordered).toHaveLength(1); + expect(ordered[0].objects?.[0]?.name).toBe('crm_account'); + }); + + it('hands back the caller\'s own body — no defaults applied, no keys stripped', () => { + // A parsed clone would arrive carrying `defaultDatasource: 'default'` and + // `scope: 'project'`, and would have dropped keys `ManifestSchema` does not + // declare. Registering that instead of the authored body is what would make + // the `packages` branch and the `manifest` branch disagree (D7). + const body = basePackage(); + const ordered = resolveArtifactPackageOrder({ packages: [{ manifest: body }] }) as PackageBody[]; + expect(ordered[0]).toBe(body); + expect(ordered[0].defaultDatasource).toBeUndefined(); + expect(ordered[0].scope).toBeUndefined(); + }); +}); + +describe('ADR-0130 D7 — an existing single-`manifest` artifact registers bit-identically', () => { + /** + * The comparison is over REGISTRY STATE after install — the package record, + * every object FQN, every `_packageId` stamp and the namespace-owner sets — + * and not over the load path's return value, because state is what the DB, the + * API and every read path see. D7 states it in exactly those terms. + * + * The reference side is a direct `engine.registerApp(payload)`: that IS the + * single call the load path made before ADR-0130, so "unchanged" is measured + * against the pre-change behaviour rather than against a second copy of the + * new behaviour. + */ + const snapshot = (ql: ObjectQL) => { + const registry = ql.registry; + const objects: Record = {}; + for (const fqn of registry.getAllObjects().map((o) => o.name).sort()) { + const resolved = registry.resolveObject(fqn) as ResolvedObject | undefined; + objects[fqn] = { + body: resolved, + packageId: resolved?._packageId, + owner: registry.getObjectOwner(fqn)?.packageId, + contributors: registry + .getObjectContributors(fqn) + .map((c) => ({ packageId: c.packageId, ownership: c.ownership, priority: c.priority, namespace: c.namespace })), + }; + } + return { + packages: registry.getAllPackages().map((p) => ({ + id: p.manifest?.id, + manifest: p.manifest, + status: p.status, + enabled: p.enabled, + })), + objectFqns: Object.keys(objects), + objects, + namespaces: ['crm', 'cpq', 'base'].map((ns) => [ns, registry.getNamespaceOwners(ns)]), + types: registry.getRegisteredTypes().sort(), + }; + }; + + const bootKernel = async () => { + const kernel = new ObjectKernel({ logger: { level: 'silent' }, gracefulShutdown: false }); + await kernel.use(new ObjectQLPlugin()); + await kernel.bootstrap(); + return kernel; + }; + + it('produces the same registry state as the pre-ADR-0130 direct registerApp', async () => { + const payload = basePackage(); + + // Two IDENTICALLY booted kernels, so the platform packages a boot installs + // are on both sides and the ONLY difference between them is which call + // registers the artifact. + const referenceKernel = await bootKernel(); + const loadPathKernel = await bootKernel(); + try { + // The reference: the single `engine.registerApp(payload)` this load path + // made before ADR-0130 — i.e. today's behaviour, not a second copy of the + // new behaviour. + const before = engineOf(referenceKernel); + before.registerApp(payload); + + // The new load path, given the same single-`manifest` artifact. + await (loadPathKernel.getService('manifest') as ManifestService).register(payload); + const after = engineOf(loadPathKernel); + + const a = snapshot(before); + const b = snapshot(after); + + // Named first so a failure reads as the fact that broke, not as a wall of + // JSON: these are D7's four enumerated comparison points. + expect(b.packages).toEqual(a.packages); + expect(b.objectFqns).toEqual(a.objectFqns); + expect(b.objectFqns.map((f) => b.objects[f].packageId)) + .toEqual(a.objectFqns.map((f) => a.objects[f].packageId)); + expect(b.namespaces).toEqual(a.namespaces); + // …and then the whole of it, so a fifth thing that moves is not missed + // just because D7 enumerated four. + expect(b).toEqual(a); + } finally { + if (referenceKernel.getState() === 'running') await referenceKernel.shutdown(); + if (loadPathKernel.getState() === 'running') await loadPathKernel.shutdown(); + } + }); + + it('reads a bare manifest through the singular branch by reference, unchanged', () => { + const payload = basePackage(); + const ordered = resolveArtifactPackageOrder(payload) as PackageBody[]; + expect(ordered).toHaveLength(1); + // Identity, not deep equality: the D4 fallback must not copy, normalize or + // re-validate the body every artifact built to date carries. + expect(ordered[0]).toBe(payload); + }); +}); diff --git a/packages/objectql/src/artifact-packages.ts b/packages/objectql/src/artifact-packages.ts new file mode 100644 index 0000000000..36f57969c8 --- /dev/null +++ b/packages/objectql/src/artifact-packages.ts @@ -0,0 +1,232 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * ADR-0130 D4 + D5 — reading a release artifact's package list, and ordering it. + * + * A release artifact MAY carry N package manifests (ADR-0130 D1: everything + * inside one artifact is delivered atomically by one publisher, and that joint + * delivery IS the co-ownership declaration). This module is the ONE place that + * turns an artifact — either shape — into the ordered list of manifests the + * load path registers. + * + * ## Both shapes are read (D4), and the fallback is the compatibility mechanism + * + * - `packages` present → iterate it. + * - `packages` absent → treat `manifest` (singular) as a **single-element list**. + * + * The second branch is not a convenience: it is the term ADR-0130's whole + * compatibility claim rests on (D7 — an existing single-`manifest` artifact must + * register bit-identically through this path). That is why this function returns + * the caller's ORIGINAL object in that branch rather than a copy or a + * re-validated clone: the bytes `registerApp` receives must not move. + * + * ## The wrapper shape is NOT re-derived here (ADR-0116's lesson) + * + * `ArtifactPackageEntrySchema` (`@objectstack/spec`, `stack.zod.ts`) is the sole + * declaration of what one entry looks like — a wrapper object carrying the + * manifest under `manifest:`, the structural position D4 reserves so a future + * `{ ref, integrity }` external segment is an additive key rather than a + * reshape. This module imports and applies that schema instead of duck-typing + * the wrapper: a second declaration of one shape is exactly the drift ADR-0116 + * exists about. + * + * ⛔ The schema is consulted as a **gate on the WRAPPER, and only the wrapper**, + * and the body handed to `registerApp` is the caller's original + * `entry.manifest`, never a parsed clone. Two measured reasons, both load-bearing: + * + * 1. **A parsed clone is not the authored body.** `ManifestSchema` carries + * defaults (`defaultDatasource: 'default'`, `scope: 'project'`) and Zod + * strips undeclared keys, so registering `parsed.data.manifest` would put + * different bytes into the registry than the singular-`manifest` branch does + * for the same authored package. D7 pins that those two branches do not + * disagree. + * 2. **`ManifestSchema` cannot express an assembled package body.** Its + * `objects` key is `z.array(z.string())` — GLOB PATTERNS (`manifest.zod.ts`) + * — while what reaches this load path is an assembled payload whose + * `objects` are object DEFINITIONS (`AppPlugin` flattens the artifact into + * `{ ...bundle.manifest, ...bundle }` before `manifest.register()`, and + * `ObjectQL.registerApp` iterates those bodies). Measured against the + * landed schema: `ArtifactPackageEntrySchema.safeParse` on such a payload + * fails with `manifest.objects.0: expected string, received object`. + * Refusing on that would refuse exactly the artifacts this path exists to + * register. + * + * So issues INSIDE the manifest body are not this seam's verdict to give — body + * validation lives at the authoring/publish doors (`defineStack`, `os validate`, + * `os compile`'s `ObjectStackDefinitionSchema.safeParse`), which is also where + * the singular-`manifest` branch has always had it. What this seam does own is + * the wrapper: an entry must be `{ manifest: … }`. ⚠️ That the entry schema's + * body half cannot describe the payload the load path registers is a real + * tension in the landed D4 surface, recorded on the card rather than papered + * over here — widening it is a spec decision, not a loader's. + * + * ## Ordering reuses the ONE sorter (D5) + * + * Packages inside an artifact MUST register in dependency-topological order: a + * package using `defineObjectExtension` to extend another package's object + * registers after the package it extends. `resolvePluginOrder` + * (`@objectstack/core`, `plugin-order.ts`) is the platform's single topological + * sorter and ADR-0116 already established that ordering is a **declared** + * contract resolved from `dependencies` — the failure mode that record exists + * for being precisely "correctness rode on which array slot each caller put it + * in". Sorting intra-artifact packages with a second, parallel implementation + * would re-create that failure inside the artifact after ADR-0116 removed it + * between packages. ⛔ Do not add a second sort here, in any form. + * + * ### Why declared dependencies enter as `optionalDependencies` + * + * `manifest.dependencies` is documented in `ManifestSchema` as a "Map of package + * IDs to version requirements", and its own example is `@steedos/plugin-auth` — + * an EXTERNAL package, resolved by the installer, definitionally not inside this + * artifact. The artifact is not the resolution scope for those: the sort's node + * set is the artifact's own package set, so a declared id that names a package + * in this artifact is a real edge and one that does not is simply not an edge + * here. That is `resolvePluginOrder`'s `optionalDependencies` semantics + * verbatim — "hoisted ahead when composed, silently skipped when absent" — so + * the classification is expressed by choosing the sorter's existing bucket + * rather than by filtering the list first and re-implementing the same rule. + * + * Reading every declared id as a HARD edge instead would refuse, at load time, + * every artifact whose manifest declares a dependency on any package outside it + * — which is every real artifact, and which D7 forbids outright. The same + * reading already exists one layer up and is written down there: the metadata + * protocol's package-scope closure keeps an unresolvable declared dependency in + * the closure and simply stops walking (`protocol.ts`, + * `resolveWritePackageScope`) rather than treating it as a fault. + * + * Cycles keep the sorter's behaviour untouched: two packages in one artifact + * that depend on each other throw, because an optional dependency is a real + * edge whenever both sides are composed. ⛔ Neither the cycle throw nor the + * missing-dependency semantics are re-adjudicated here. + */ + +import { resolvePluginOrder, type OrderablePlugin } from '@objectstack/core'; +import { ArtifactPackageEntrySchema } from '@objectstack/spec'; + +/** + * Refusals raised by {@link resolveArtifactPackageOrder}, as ADR-0112 envelopes + * (`code` + `status`) — the shape this repository's rejection tests assert + * against, never a bare throw. + */ +export type ArtifactPackageError = Error & { code: string; status: number }; + +function refuse(code: string, message: string): ArtifactPackageError { + const err = new Error(message) as ArtifactPackageError; + err.code = code; + err.status = 422; + return err; +} + +/** The ordering-relevant projection of one artifact package. */ +interface ArtifactPackageNode extends OrderablePlugin { + /** The caller's ORIGINAL manifest body — never a parsed clone. */ + manifest: unknown; +} + +/** + * Resolve an artifact into the manifests to register, in dependency-topological + * order (ADR-0130 D4 + D5). + * + * @param artifact - A release artifact (`{ packages: [...] }`), or a bare + * manifest / single-`manifest` artifact — both shapes are read. + * @returns The manifest bodies to register, in the order to register them. + * @throws An ADR-0112 envelope (`code` + `status: 422`) for a malformed entry or + * a duplicate package id, and `resolvePluginOrder`'s own error for a cycle. + */ +export function resolveArtifactPackageOrder(artifact: unknown): unknown[] { + const declared = (artifact as { packages?: unknown } | null | undefined)?.packages; + + // D4, second branch: no `packages` key → the artifact carries one package and + // the caller's own object IS that package's manifest body. Returned by + // reference, unvalidated and unrewritten — this is the path every artifact + // built to date takes, and D7 pins that it did not move. + if (declared === undefined || declared === null) return [artifact]; + + if (!Array.isArray(declared)) { + throw refuse( + 'INVALID_ARTIFACT_PACKAGES', + 'A release artifact\'s `packages` must be an array of package entries ' + + '(ADR-0130 D4, `ArtifactPackageEntrySchema`), but this artifact carries ' + + `\`packages\` of type ${typeof declared}. Omit the key entirely for a ` + + 'single-package artifact — `manifest` is retained, not replaced.', + ); + } + + const nodes = new Map(); + + declared.forEach((entry: unknown, index: number) => { + // The wrapper contract, read off its ONE declaration rather than + // duck-typed. The mistake this catches is the one the schema's own + // `history` text exists for: a manifest body inlined straight onto the + // array element instead of wrapped as `{ manifest: { … } }`. + // + // WRAPPER-LEVEL issues only — an issue at the entry root (`strictObject`'s + // `unrecognized_keys` for an inlined body, or a non-object entry) or on + // `manifest` itself (absent, or not an object). Issues DEEPER than that + // describe the manifest body, which this seam deliberately does not judge; + // see the module header for the measurement behind that line. + const verdict = ArtifactPackageEntrySchema.safeParse(entry); + const wrapperIssues = verdict.success + ? [] + : verdict.error.issues.filter( + (i) => i.path.length === 0 || (i.path.length === 1 && i.path[0] === 'manifest'), + ); + if (wrapperIssues.length > 0) { + throw refuse( + 'INVALID_ARTIFACT_PACKAGE_ENTRY', + `Release artifact \`packages[${index}]\` is not a package entry (ADR-0130 D4): ` + + wrapperIssues.map((i) => `${i.path.join('.') || ''}: ${i.message}`).join('; ') + + '. Each entry is a WRAPPER object carrying its package under `manifest:` — ' + + 'wrap an inlined body as `{ manifest: { … } }`. The key position is reserved ' + + 'so a future external-segment form is an additive key rather than a reshape.', + ); + } + + // ⛔ The ORIGINAL body, never `verdict.data.manifest` — see the module + // header: the schema is a gate here, and a parsed clone carries defaults + // and drops undeclared keys the singular-`manifest` branch keeps. + const manifest = (entry as { manifest?: unknown }).manifest; + // `||`, not `??`, on purpose: `ObjectQL.registerApp` keys the installed + // package on `manifest.id || manifest.name`, so an empty-string `id` falls + // back to `name` there — this seam must agree on what the id IS, or the + // sorter would order a package under a key the registry never stores it by. + const id = (manifest as { id?: unknown; name?: unknown }).id + || (manifest as { name?: unknown }).name; + + if (typeof id !== 'string' || id === '') { + throw refuse( + 'INVALID_ARTIFACT_PACKAGE_ENTRY', + `Release artifact \`packages[${index}]\` carries a manifest with no usable ` + + 'package id: `registerApp` keys the installed package on `id || name`, so ' + + 'an entry without either cannot be ordered against its siblings or ' + + 'addressed after install.', + ); + } + + if (nodes.has(id)) { + // Deduplicating silently is the failure this refusal exists to prevent: + // one of the two bodies would simply never register, and nothing + // downstream could tell that it had been dropped. + throw refuse( + 'DUPLICATE_ARTIFACT_PACKAGE', + `Release artifact declares package "${id}" more than once (\`packages[${index}]\` ` + + 'repeats an earlier entry). One artifact carries each package once — an ' + + 'artifact is one atomic delivery (ADR-0130 D1/D6), not a list with ' + + 'last-writer-wins.', + ); + } + + nodes.set(id, { + // `name` is what `resolvePluginOrder` puts in its diagnostics; the MAP KEY + // is what its edges resolve against. Both are the package id, so an error + // it raises names the same string the artifact author wrote. + name: id, + optionalDependencies: Object.keys( + (manifest as { dependencies?: Record }).dependencies ?? {}, + ), + manifest, + }); + }); + + return resolvePluginOrder(nodes).map((node) => node.manifest); +} diff --git a/packages/objectql/src/index.ts b/packages/objectql/src/index.ts index b503262b2b..2c21499463 100644 --- a/packages/objectql/src/index.ts +++ b/packages/objectql/src/index.ts @@ -409,3 +409,11 @@ export type { } from './write-epoch.js'; export { bridgeAuthzInvalidation } from './authz-invalidation-bridge.js'; export type { AuthzInvalidationBridgeOptions } from './authz-invalidation-bridge.js'; + +// [ADR-0130 D4/D5] Reading a release artifact's package list, and ordering it. +// The load path in `plugin.ts` consumes this; it is exported so every other +// door that grows an artifact-loading seam (the CLI, the marketplace install +// path, the metadata dev-artifact loader) adopts the SAME read of both shapes +// and the SAME single sorter, instead of each re-deriving one. +export { resolveArtifactPackageOrder } from './artifact-packages.js'; +export type { ArtifactPackageError } from './artifact-packages.js'; diff --git a/packages/objectql/src/plugin.ts b/packages/objectql/src/plugin.ts index b3cbf1ad4c..35a752dcaf 100644 --- a/packages/objectql/src/plugin.ts +++ b/packages/objectql/src/plugin.ts @@ -4,6 +4,7 @@ import { ObjectQL } from './engine.js'; import { assembleMetadataProtocol } from '@objectstack/metadata-protocol'; import type { MetadataAuthoringChannel } from '@objectstack/metadata-protocol'; import { Plugin, PluginContext } from '@objectstack/core'; +import { resolveArtifactPackageOrder } from './artifact-packages.js'; import { applyConversionsToStoredItem } from '@objectstack/spec'; import { StorageNameMapping } from '@objectstack/spec/system'; import { LifecycleService } from './lifecycle/lifecycle-service.js'; @@ -401,20 +402,52 @@ export class ObjectQLPlugin implements Plugin { // instead of the legacy ctx.registerService('app.', manifestData) convention. const ql = this.ql; ctx.registerService('manifest', { - register: (manifest: any) => { - ql.registerApp(manifest); - ctx.logger.debug('Manifest registered via manifest service', { - id: manifest.id || manifest.name - }); + // [ADR-0130 D4/D5] The load path reads BOTH artifact shapes and registers + // the packages inside one artifact in dependency-TOPOLOGICAL order. + // + // `resolveArtifactPackageOrder` is the whole of that decision: it reads + // `packages` when present and falls back to the caller's own object as a + // single-element list when absent (D4), and it orders through + // `resolvePluginOrder` — the platform's ONE topological sorter (ADR-0116). + // ⛔ Nothing here may re-derive either the entry shape or the ordering; a + // second copy of either is the drift both records exist about. + // + // Registration of ALL packages completes before ANY bridging begins. The + // bridge resolves objects out of the registry, and an artifact's later + // packages contribute extensions onto the earlier ones' objects — so a + // register/bridge interleave would bridge a body that the very next + // package is about to change. For the single-package artifact this is the + // same one register followed by the same one bridge as before (D7). + register: (artifact: any) => { + const ordered = resolveArtifactPackageOrder(artifact) as any[]; + + if (ordered.length === 0) { + // An artifact that declared `packages: []` registers nothing. Said out + // loud rather than returning quietly: "the install did nothing" is not + // a state anyone should have to infer from an absence. + ctx.logger.warn( + 'Artifact declared an empty `packages: []` — nothing was registered. A ' + + 'single-package artifact omits the key entirely and carries `manifest` ' + + '(ADR-0130 D4).', + ); + return Promise.resolve(); + } + + for (const manifest of ordered) { + ql.registerApp(manifest); + ctx.logger.debug('Manifest registered via manifest service', { + id: manifest.id || manifest.name + }); + } // Manifests registered AFTER start() (marketplace install / ledger // rehydrate arrive on `kernel:ready` or an HTTP request) land in the // SchemaRegistry only — the one-shot startup bridge already ran — so - // bridge this manifest's objects into the metadata service now. + // bridge every registered package's objects into the metadata service now. // No-op until start() arms it, so boot-time registrations keep the // single startup bridge. The promise never rejects; async callers // (marketplace install) await it so metadata reads right after // install are deterministic, sync callers may ignore it. - return this.bridgeManifestObjectsToMetadataService(ctx, manifest); + return this.bridgeArtifactObjectsToMetadataService(ctx, ordered); } }); @@ -1873,6 +1906,26 @@ export class ObjectQLPlugin implements Plugin { } } + /** + * [ADR-0130 D5] Bridge each package of one artifact, in the SAME + * dependency-topological order they registered in. + * + * Sequential, not `Promise.all`: {@link bridgeManifestObjectsToMetadataService} + * reads an object out of the metadata service and decides whether the copy + * sitting there is its own before overwriting it, so two packages + * contributing to one object must not interleave that read-then-write. For an + * artifact carrying one package this awaits exactly the one bridge the + * pre-ADR-0130 path returned (D7). + */ + private async bridgeArtifactObjectsToMetadataService( + ctx: PluginContext, + manifests: any[], + ): Promise { + for (const manifest of manifests) { + await this.bridgeManifestObjectsToMetadataService(ctx, manifest); + } + } + /** * Bridge ONE manifest's objects into the metadata service — the * late-registration companion to {@link bridgeObjectsToMetadataService}. diff --git a/packages/runtime/src/dispatcher-error-vocabulary.ts b/packages/runtime/src/dispatcher-error-vocabulary.ts index 738739f6d2..29c9809840 100644 --- a/packages/runtime/src/dispatcher-error-vocabulary.ts +++ b/packages/runtime/src/dispatcher-error-vocabulary.ts @@ -649,6 +649,73 @@ export const UNREGISTERED_CODE_SITES: readonly UnregisteredCodeSite[] = [ 'vocabulary. If a transport ever ANSWERS with this fact, the verdict becomes ' + 'pending-registration and the code belongs in the ledger batch.', }, + // [ADR-0130 D4] The artifact load path's three wrapper refusals, added with the + // N-package load path itself. The pre-HTTP reasoning is the one the rows above + // cite; what is specific to these three is the second half recorded in each `why` + // — the one door that can reach them catches them and answers with its own + // registered code, so they are not demoted at a door, they never reach one. + { + code: 'INVALID_ARTIFACT_PACKAGES', + file: 'packages/objectql/src/artifact-packages.ts', + shape: 'codehelper', + door: 'none', + verdict: 'boot-refusal', + why: + 'The refusal for an artifact whose `packages` key is present but not an array. Raised by ' + + '`resolveArtifactPackageOrder`, which the `manifest` service calls on every `register()` ' + + '(ADR-0130 D4/D5). Measured on this tree, that service has three callers and none of them puts ' + + 'this code on a wire: `packages/runtime/src/app-plugin.ts` registers at boot inside plugin init, ' + + 'where a throw aborts boot before any HTTP boundary exists; the rehydrate loop in ' + + '`packages/cloud-connection/src/marketplace-install-local-plugin.ts` catches per entry and logs; ' + + 'and the HTTP install route in that same file catches and answers with its OWN registered ' + + '`PLUGIN_REGISTER_FAILED` at 422, interpolating this refusal\'s MESSAGE into that envelope. So the ' + + 'code reaches a reader only inside a message string, never as `error.code` — not demoted at a ' + + 'door, absent from the wire entirely. Its `status: 422` is the ADR-0112 envelope shape this ' + + '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.' + }, + { + code: 'INVALID_ARTIFACT_PACKAGE_ENTRY', + file: 'packages/objectql/src/artifact-packages.ts', + shape: 'codehelper', + door: 'none', + verdict: 'boot-refusal', + why: + 'The refusal for a `packages[]` element that is not a `{ manifest: … }` wrapper, or whose ' + + 'manifest carries no usable package id. Raised by `resolveArtifactPackageOrder`, which the ' + + '`manifest` service calls on every `register()` (ADR-0130 D4/D5). Measured on this tree, that ' + + 'service has three callers and none of them puts this code on a wire: ' + + '`packages/runtime/src/app-plugin.ts` registers at boot inside plugin init, where a throw aborts ' + + 'boot before any HTTP boundary exists; the rehydrate loop in ' + + '`packages/cloud-connection/src/marketplace-install-local-plugin.ts` catches per entry and logs; ' + + 'and the HTTP install route in that same file catches and answers with its OWN registered ' + + '`PLUGIN_REGISTER_FAILED` at 422, interpolating this refusal\'s MESSAGE into that envelope. So the ' + + 'code reaches a reader only inside a message string, never as `error.code` — not demoted at a ' + + 'door, absent from the wire entirely. Its `status: 422` is the ADR-0112 envelope shape this ' + + '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.' + }, + { + code: 'DUPLICATE_ARTIFACT_PACKAGE', + file: 'packages/objectql/src/artifact-packages.ts', + shape: 'codehelper', + door: 'none', + verdict: 'boot-refusal', + why: + 'The refusal for one artifact declaring the same package id twice — raised rather than ' + + 'deduplicated so a dropped body cannot go unnoticed. Raised by `resolveArtifactPackageOrder`, ' + + 'which the `manifest` service calls on every `register()` (ADR-0130 D4/D5). Measured on this ' + + 'tree, that service has three callers and none of them puts this code on a wire: ' + + '`packages/runtime/src/app-plugin.ts` registers at boot inside plugin init, where a throw aborts ' + + 'boot before any HTTP boundary exists; the rehydrate loop in ' + + '`packages/cloud-connection/src/marketplace-install-local-plugin.ts` catches per entry and logs; ' + + 'and the HTTP install route in that same file catches and answers with its OWN registered ' + + '`PLUGIN_REGISTER_FAILED` at 422, interpolating this refusal\'s MESSAGE into that envelope. So the ' + + 'code reaches a reader only inside a message string, never as `error.code` — not demoted at a ' + + 'door, absent from the wire entirely. Its `status: 422` is the ADR-0112 envelope shape this ' + + '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.' + }, // ── [#13233] field-level catalogs, reached by the OBJECT-LITERAL helper ── // // The 29 rows below are the whole verdict cost of widening `codehelper` to