diff --git a/packages/cli/src/utils/artifact-packages.test.ts b/packages/cli/src/utils/artifact-packages.test.ts new file mode 100644 index 0000000000..ce32ab6093 --- /dev/null +++ b/packages/cli/src/utils/artifact-packages.test.ts @@ -0,0 +1,146 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [ADR-0130 D4 — #18204] The per-package leg's resolution context, asserted + * against the FUNCTION the commands call. + * + * `compile.ts` (step 3b-ii) and `lint.ts` both build one package's stack with + * {@link packageBodyAsStack}, handing it the artifact's own `packages[]` so a + * reference into a SIBLING package's object resolves (#16611). Two neighbours + * already measure parts of that, and neither covers what this file does: + * + * - `packages/lint/src/validate-object-references.test.ts` runs the rule + * against a local three-key REPLICA of this function's output. It stays + * green if the CLI stops producing that shape — which is the only way this + * defect can ever return. + * - `test/build-multi-package-artifact.e2e.test.ts` runs the real command end + * to end, and is the stronger reading — but it carries the `*.e2e` name, so + * `OS_TEST_TIERS` puts it in the NIGHTLY population (`../../vitest-tiers.ts`) + * and the merge queue never runs it. A regression lands green and is found + * the next morning. + * + * So this file is the queue-tier half: the real `artifactPackages` + + * `packageBodyAsStack`, the real rule, and no replica anywhere in the chain. + * + * ## The two LEVELS, which is the card + * + * #18204 reported one reference accepted at the field level and the same name + * refused at the action-param level in the same build. That is what + * `@objectstack/cli@17.4.0` does — its `packageBodyAsStack(body)` takes no + * context argument at all, so the action-param site is judged against one + * package's objects while the field site is not judged at all by that release's + * rule. Both halves moved since; this pins them MOVED TOGETHER, because a + * platform that accepts `Field.lookup('x')` and refuses `reference: 'x'` on an + * action param is contradicting itself inside one command. + * + * ⛔ Not a "skip the site per package" pin. The dangling name is asserted + * present at BOTH levels in the same call, so a context that silences the + * ladder fails here exactly as loudly as a context that never arrives. + */ + +import { describe, expect, it } from 'vitest'; +import { validateObjectReferences } from '@objectstack/lint'; + +import { artifactPackages, packageBodyAsStack } from './artifact-packages.js'; + +/** + * A two-package artifact in the shape `composeStacks(…, { manifest: 'preserve' })` + * writes and `ObjectStackDefinitionSchema` parses: each entry's assembled body + * under `manifest`. Modelled on `examples/app-multi-package`, whose `orders` + * package reads `crm_account` out of its `core` sibling. + */ +const CORE_BODY = { + id: 'com.example.multi.core', + objects: [{ name: 'crm_account', fields: { name: { type: 'text' } } }], +}; + +const ORDERS_BODY = { + id: 'com.example.multi.orders', + dependencies: { 'com.example.multi.core': '^1.0.0' }, + objects: [ + { + name: 'crm_order', + fields: { + name: { type: 'text' }, + // Level 1 — `Field.lookup()`'s target, accepted across packages by + // ADR-0130 §1.5. + account: { type: 'lookup', reference: 'crm_account' }, + ghost: { type: 'lookup', reference: 'crm_nowhere' }, + }, + actions: [ + { + name: 'link_account', + type: 'script', + locations: [], + params: [ + // Level 2 — the record picker's search target, the card's level. + { name: 'account', type: 'lookup', reference: 'crm_account' }, + { name: 'phantom', type: 'lookup', reference: 'crm_nowhere_param' }, + ], + body: { language: 'js', source: 'return { ok: true };', capabilities: [] }, + }, + ], + }, + ], +}; + +const ARTIFACT = { + manifest: { id: 'com.example.multi.core' }, + objects: CORE_BODY.objects, + packages: [{ manifest: ORDERS_BODY }, { manifest: CORE_BODY }], +} as Record; + +/** + * Exactly what `compile.ts` step 3b-ii does for one entry. + * + * ⛔ `context` takes no DEFAULT: a default would be applied to the CONTROL + * leg's explicit `undefined` — the exact value the pre-#16611 call site passes + * — and that leg would silently measure the fixed shape and pass. + */ +const perPackageStack = (index: number, context: unknown) => + packageBodyAsStack(artifactPackages(ARTIFACT)[index].body, context); + +describe('packageBodyAsStack — the per-package leg resolves a sibling package at BOTH reference levels (#18204)', () => { + it('CONTROL — with no context the SAME body errors at both levels, so the context is what does the work', () => { + // Without this leg a green assertion below is indistinguishable from a rule + // that stopped judging these sites at all. + const paths = validateObjectReferences(perPackageStack(0, undefined)).map((f) => f.path); + expect(paths).toEqual([ + 'objects[0].fields.account.reference', + 'objects[0].fields.ghost.reference', + 'objects[0].actions[0].params[0].reference', + 'objects[0].actions[0].params[1].reference', + ]); + }); + + it('resolves the sibling\'s object at both levels, and still refuses a name no package provides', () => { + const findings = validateObjectReferences(perPackageStack(0, ARTIFACT.packages)); + // One equality carries all four verdicts: `crm_account` is absent at BOTH + // levels (resolved through `packages[]`) and the two dangling names are + // present at BOTH levels (the refusal MOVED — it did not disappear). + expect(findings.map((f) => f.path)).toEqual([ + 'objects[0].fields.ghost.reference', + 'objects[0].actions[0].params[1].reference', + ]); + expect(findings.map((f) => f.rule)).toEqual([ + 'object-reference-unknown', + 'object-reference-unknown', + ]); + expect(findings.map((f) => f.severity)).toEqual(['error', 'error']); + }); + + it('judges the sibling from its own side unchanged', () => { + // The context widens what a package can RESOLVE, never what it JUDGES: the + // app package owns no reference, so it has nothing to report either way. + expect(validateObjectReferences(perPackageStack(1, ARTIFACT.packages))).toEqual([]); + }); + + it('hands the rule the artifact\'s own entries — the body of each package is under `manifest`', () => { + // The shape is the contract between this function and the rule: the rule + // reads `packages[].manifest.`, so a context reduced to some + // other entry shape resolves nothing while every type still checks. + const stack = perPackageStack(0, ARTIFACT.packages) as { packages?: unknown; manifest?: unknown }; + expect(stack.packages).toBe(ARTIFACT.packages); + expect(stack.manifest).toBe(artifactPackages(ARTIFACT)[0].body); + }); +}); diff --git a/packages/cli/test/build-multi-package-artifact.e2e.test.ts b/packages/cli/test/build-multi-package-artifact.e2e.test.ts index ccc11753be..adb2703113 100644 --- a/packages/cli/test/build-multi-package-artifact.e2e.test.ts +++ b/packages/cli/test/build-multi-package-artifact.e2e.test.ts @@ -143,15 +143,31 @@ export default { * between the ruled fix and "skip the site per package". * * One fixture measures both, because the discriminating fact is WHICH of the - * two lookups on `probe_order` is reported: `account` (a sibling provides it) - * must not be, `ghost` (nothing provides it) must be. Dropping the `packages[]` + * lookups on `probe_order` is reported: `account` (a sibling provides it) must + * not be, `ghost` (nothing provides it) must be. Dropping the `packages[]` * pass-through reports both; dropping the judging reports neither. * + * ## Two LEVELS, one artifact, one verdict (#18204) + * + * `probe_order` carries the same pair of names twice: once as a field + * `reference` (`Field.lookup()`'s target) and once as an action param's + * record-picker `reference`. Both are resolved by the same ladder in + * `validate-object-references.ts` against the same set, and ADR-0130 §1.5 + * accepts a cross-package lookup — so a build in which one level resolves a + * sibling's object and the other refuses it is the platform contradicting + * itself inside a single command. That contradiction SHIPPED: `@objectstack/ + * cli@17.4.0` lowers a package body with `packageBodyAsStack(body)` and no + * context argument at all, which refuses the action-param reference while the + * field level — a site that release's rule did not yet walk — goes unjudged + * and therefore looks accepted. The equality below is what keeps the two + * levels from drifting apart again: it names the paths of BOTH refusals and, + * by their absence, BOTH resolutions. + * * ⚠️ The top level carries `probe_account`, so `objects` is PRESENT and * `authoringRuleUnionStack` folds nothing into it — it only ever fills ABSENT * keys, which `src/utils/stack-collections.test.ts` pins BY IDENTITY on exactly * this shape (a stack carrying both its collections and `packages[]`). So the - * union run never sees `probe_order`'s fields, and this is a pin on + * union run never sees `probe_order`'s fields or actions, and this is a pin on * `compile.ts`'s half rather than a second copy of `packages/lint`'s rule test, * whose input is a local three-key REPLICA of `packageBodyAsStack` * (`perPackageStack`) and stays green if this command stops building that @@ -160,8 +176,9 @@ export default { * ⭐ That last claim is MEASURED rather than argued. Three ablations, each * reddening THIS case alone and leaving the other six in this file green: * - * 1. drop `packages[]` from `packageBodyAsStack` (the pre-#16611 shape) — the - * equality below receives BOTH paths, `account` first; + * 1. drop `packages[]` from `packageBodyAsStack` (the pre-#16611 shape, and + * the one `@objectstack/cli@17.4.0` ships) — the equality below receives + * all FOUR paths, `account` first; * 2. hand the per-package leg no `objects` at all, i.e. the "skip the site per * package" option the ruling rejected — `os build` exits **0** with * `success: true`, so the union run is NOT a second reporter for these @@ -185,6 +202,16 @@ const probeOrder = { account: { type: 'lookup', label: 'Account', reference: 'probe_account' }, ghost: { type: 'lookup', label: 'Ghost', reference: 'probe_nothing' }, }, + actions: [ + { + name: 'probe_link', label: 'Link', type: 'script', locations: [], + params: [ + { name: 'account', label: 'Account', type: 'lookup', reference: 'probe_account' }, + { name: 'phantom', label: 'Phantom', type: 'lookup', reference: 'probe_nothing_param' }, + ], + body: { language: 'js', source: 'return { ok: true };', capabilities: [] }, + }, + ], }; export default { @@ -289,7 +316,7 @@ describe('ADR-0130 D4 — `os build` emits one artifact carrying `packages[]`', expect(paths).toContain('packages.0.manifest.objects.0'); }, 180_000); - it('resolves a SIBLING package\'s object on the per-package leg, and still errors on an artifact-wide dangling one', async () => { + it('resolves a SIBLING package\'s object on the per-package leg at BOTH reference levels, and still errors on an artifact-wide dangling one', async () => { const run = await runCli(['build', '--json'], dirs.refs); expect(run.code, `${run.stdout}\n${run.stderr}`).toBe(1); const payload = JSON.parse(run.stdout) as { @@ -301,11 +328,29 @@ describe('ADR-0130 D4 — `os build` emits one artifact carrying `packages[]`', // is the only thing that distinguishes the two exits from outside. expect(payload.error).toBe('author-time rules failed for one or more packages'); const refs = (payload.issues ?? []).filter((i) => i.rule === 'object-reference-unknown'); - // Both directions in one equality: `ghost` is present (the leg still - // JUDGES) and `account` is absent (the leg RESOLVED it through the - // artifact's `packages[]`). A pass-through that went missing reports both. - expect(refs.map((i) => i.path)).toEqual(['objects[0].fields.ghost.reference']); - expect(refs[0].package).toBe('com.example.probe.orders'); + // Every direction in ONE equality, over BOTH levels (#18204): + // + // present — `fields.ghost` and `params[1]` (`probe_nothing_param`): the + // leg still JUDGES both levels, so the refusal MOVED rather + // than disappearing when the context was added; + // absent — `fields.account` and `params[0]`, both naming + // `probe_account`: the leg RESOLVED that name through the + // artifact's `packages[]` at the field level AND at the + // action-param level, which is the agreement the two levels + // owe each other. + // + // The order is the rule's own walk order (`validate-object-references.ts`: + // object fields, then global actions, then object-embedded actions), so an + // equality — not a `toContain` pair — is what keeps a silently dropped + // finding visible. + expect(refs.map((i) => i.path)).toEqual([ + 'objects[0].fields.ghost.reference', + 'objects[0].actions[0].params[1].reference', + ]); + expect(refs.map((i) => i.package)).toEqual([ + 'com.example.probe.orders', + 'com.example.probe.orders', + ]); }, 180_000); });