Skip to content

test(cli): hold field-level and action-param object references in agreement on the per-package leg #18213

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
os-elon-musk merged 1 commit into from
Sep 15, 2026
+202 −11
Merged

test(cli): hold field-level and action-param object references in agreement on the per-package leg #18213

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
146 changes: 146 additions & 0 deletions packages/cli/src/utils/artifact-packages.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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<string, unknown>;

/**
* 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.<collection>`, 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);
});
});
67 changes: 56 additions & 11 deletions packages/cli/test/build-multi-package-artifact.e2e.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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 {
Expand Down Expand Up @@ -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 {
Expand All @@ -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);
});

Expand Down
Loading