Skip to content

Commit fb3c6b4

Browse files
os-elon-muskclaude
andauthored
test(cli): hold field-level and action-param object references in agreement on the per-package leg (#18213)
Fixes #18204 Part of #14122 — the ADR-0130 tracker stays open; nothing here is a closing keyword for it. ## Headline: the reported defect does not reproduce on `main` — the missing thing was the pin The card reports that `os build`'s per-package author-time leg refuses an action param's record-picker `reference` into a dependency's object while the composed pass accepts it, and while ADR-0130 R1 accepts the field-level equivalent. Measured on this tree, both levels agree and have since #16611: the per-package leg resolves the sibling's object at the field level AND at the action-param level, and still refuses a name no package of the artifact provides. What DOES reproduce the card, exactly, is the published `@objectstack/cli@17.4.0` that the downstream repro builds against. Read out of that installed tarball: ```js // node_modules/@objectstack/cli/dist/commands/compile.js (17.4.0, as installed downstream) function packageBodyAsStack(body) { return { ...body, manifest: body }; // no resolution context at all } ``` and its `@objectstack/lint@17.4.0` carries neither `artifactProvidedObjectNames` nor the field `reference` site. That is the whole asymmetry the card saw: at that release the action-param site is judged against one package's own objects (refused), while the field site is not judged at all (so it looks accepted). Both halves moved in #16611. The downstream fix is a pin bump, not a change here. So this PR changes no behaviour. It adds the pin the card asks for — "the refusal must move, not disappear, and a fixture must prove it" — at the level the card is about. ## Measurements All on `examples/app-multi-package` (a real two-package artifact: module `orders` depends on app `core`, which owns `crm_account`), through the real `objectstack build`: | fixture | reading | | --- | --- | | unmodified example | exit 0 | | module action param `reference: 'crm_account'` (app package's object) | exit 0 — the per-package leg ran and reported nothing | | the same param pointed at `crm_contract` (no package provides it) | exit 1, `object-reference-unknown` at `objects[0].actions[0].params[0].reference` | The example edits were a throwaway probe and are not in this diff (restored; `git diff HEAD` empty). ## What the two new pins hold - `packages/cli/src/utils/artifact-packages.test.ts` (new, queue tier) — the real `artifactPackages` + `packageBodyAsStack` + the real rule. One equality carries four verdicts: `crm_account` absent at BOTH levels (resolved through `packages[]`), the two dangling names present at BOTH levels (the refusal moved, it did not disappear). A CONTROL leg with no context reports all four, so the pin can fail. - `packages/cli/test/build-multi-package-artifact.e2e.test.ts` — the existing end-to-end case for the same seam gains the action-param level, so the same four verdicts are also read through the real command. Why both: the lint-side rule test runs against a local REPLICA of `packageBodyAsStack`'s output and stays green if the CLI stops producing that shape; the end-to-end file carries the `.e2e` name, so `OS_TEST_TIERS` puts it in the NIGHTLY population and the merge queue never runs it. The new file is the queue-tier half. ### Ablation — the pins fail when the wiring is removed Mutating `packageBodyAsStack` back to the 17.4.0 shape (`{ ...body, manifest: body }`), on-disk mutation proved by text counts (removed-text 1 to 0, injected-text 1) and by blob hash (`febc6ad4` to `9b4019eb`): ``` ABLATION unit exit=1 ABLATION e2e exit=1 ``` both failing with the same diff — the two `crm_account` / `probe_account` paths appearing, including `objects[0].actions[0].params[0].reference`, which IS the card's symptom reproduced on this tree. Restored from `HEAD`: `git diff HEAD` empty, blob back to `febc6ad4`. ## Tests and gates - `pnpm --filter @objectstack/cli exec vitest run --project unit` — 207 files, 2962 tests, all pass (the new file included). - `OS_TEST_TIERS=nightly ... --project integration build-multi-package-artifact` — 7 pass. Run because the diff touches an integration-layer file. - `pnpm --filter @objectstack/cli typecheck` — exit 0. - `pnpm lint` (`eslint . --no-inline-config`, repo-wide, not narrowed) — exit 0 over 6754 files at `1a4624be8`. - `node scripts/pm/dispatch-gates.mjs --commands` derived 57 families; 55 run green, 2 recorded exit 3 (PREREQUISITE NOT MET — `check:dual-build-cjs-loads` and `check:i18n-coverage` both need a full-tree build; this worktree built only the CLI closure). Reconciled with `--ran`: 57 derived, 57 accounted, 0 UNRUN. One further self-test, `check-plugin-teardown-shape --self-test`, exits 1 on this container because the clone is shallow and it cannot reach its pinned fixture commit — its tree-judging leg passes. ## Changeset `skip-changeset`, measured rather than assumed: `packages/cli` publishes `files: ["dist", "README.md", "CHANGELOG.md"]`, the diff is two test files, and neither name appears anywhere under `packages/cli/dist` after a build, while the positive control (`packageBodyAsStack`) does. Nothing published moves. The dispatch asked for a `patch` entry describing what the per-package pass now accepts; that sentence would be false — it has accepted it since #16611, whose own changeset carried it. ## Clause 2 The claim on the card declares `Clause-2: yes` on the expectation that the fix widens the acceptance set. The measurement falsifies that premise for this PR: the accept-set is byte-identical before and after. `needs:contract-review` is attached anyway, per the claim, so the contract seat can re-grade rather than have the label quietly dropped by the executor. ## Acceptance notes Observed while measuring, not filed and not fixed here: - `compile.ts` holds two adjacent variables for the same noun — `artifactPackageEntries` (the raw `packages[]`, which is the resolution context the rule reads) and `packageEntries` (the `{ index, id, body }` view). `packageBodyAsStack`'s second parameter is typed `unknown`, so passing the wrong one type-checks and silently restores exactly this refusal. The end-to-end and queue-tier pins above now catch it; the type does not. Carrier: whoever next refactors that block. - `objectExtensions[].fields[].reference` is a third level with a third verdict — not judged at all, deliberately and documented in `validate-object-references.ts`, whose header names the cross-artifact case as its own card. Reported per the dispatch's ruling 2 rather than widened into. - `os validate` runs the union pass only, so it has no per-package leg to disagree with; `os lint`'s per-package leg passes the same raw `packages[]` this one does; `os dev` spawns `compile`. No fourth verdict found. --- _Generated by [Claude Code](https://claude.ai/code/session_01T3YsvpK1PvYf9n1YUhYP6W)_ Co-authored-by: Claude <noreply@anthropic.com>
1 parent 8c657f7 commit fb3c6b4

2 files changed

Lines changed: 202 additions & 11 deletions

File tree

Lines changed: 146 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,146 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* [ADR-0130 D4 — #18204] The per-package leg's resolution context, asserted
5+
* against the FUNCTION the commands call.
6+
*
7+
* `compile.ts` (step 3b-ii) and `lint.ts` both build one package's stack with
8+
* {@link packageBodyAsStack}, handing it the artifact's own `packages[]` so a
9+
* reference into a SIBLING package's object resolves (#16611). Two neighbours
10+
* already measure parts of that, and neither covers what this file does:
11+
*
12+
* - `packages/lint/src/validate-object-references.test.ts` runs the rule
13+
* against a local three-key REPLICA of this function's output. It stays
14+
* green if the CLI stops producing that shape — which is the only way this
15+
* defect can ever return.
16+
* - `test/build-multi-package-artifact.e2e.test.ts` runs the real command end
17+
* to end, and is the stronger reading — but it carries the `*.e2e` name, so
18+
* `OS_TEST_TIERS` puts it in the NIGHTLY population (`../../vitest-tiers.ts`)
19+
* and the merge queue never runs it. A regression lands green and is found
20+
* the next morning.
21+
*
22+
* So this file is the queue-tier half: the real `artifactPackages` +
23+
* `packageBodyAsStack`, the real rule, and no replica anywhere in the chain.
24+
*
25+
* ## The two LEVELS, which is the card
26+
*
27+
* #18204 reported one reference accepted at the field level and the same name
28+
* refused at the action-param level in the same build. That is what
29+
* `@objectstack/cli@17.4.0` does — its `packageBodyAsStack(body)` takes no
30+
* context argument at all, so the action-param site is judged against one
31+
* package's objects while the field site is not judged at all by that release's
32+
* rule. Both halves moved since; this pins them MOVED TOGETHER, because a
33+
* platform that accepts `Field.lookup('x')` and refuses `reference: 'x'` on an
34+
* action param is contradicting itself inside one command.
35+
*
36+
* ⛔ Not a "skip the site per package" pin. The dangling name is asserted
37+
* present at BOTH levels in the same call, so a context that silences the
38+
* ladder fails here exactly as loudly as a context that never arrives.
39+
*/
40+
41+
import { describe, expect, it } from 'vitest';
42+
import { validateObjectReferences } from '@objectstack/lint';
43+
44+
import { artifactPackages, packageBodyAsStack } from './artifact-packages.js';
45+
46+
/**
47+
* A two-package artifact in the shape `composeStacks(…, { manifest: 'preserve' })`
48+
* writes and `ObjectStackDefinitionSchema` parses: each entry's assembled body
49+
* under `manifest`. Modelled on `examples/app-multi-package`, whose `orders`
50+
* package reads `crm_account` out of its `core` sibling.
51+
*/
52+
const CORE_BODY = {
53+
id: 'com.example.multi.core',
54+
objects: [{ name: 'crm_account', fields: { name: { type: 'text' } } }],
55+
};
56+
57+
const ORDERS_BODY = {
58+
id: 'com.example.multi.orders',
59+
dependencies: { 'com.example.multi.core': '^1.0.0' },
60+
objects: [
61+
{
62+
name: 'crm_order',
63+
fields: {
64+
name: { type: 'text' },
65+
// Level 1 — `Field.lookup()`'s target, accepted across packages by
66+
// ADR-0130 §1.5.
67+
account: { type: 'lookup', reference: 'crm_account' },
68+
ghost: { type: 'lookup', reference: 'crm_nowhere' },
69+
},
70+
actions: [
71+
{
72+
name: 'link_account',
73+
type: 'script',
74+
locations: [],
75+
params: [
76+
// Level 2 — the record picker's search target, the card's level.
77+
{ name: 'account', type: 'lookup', reference: 'crm_account' },
78+
{ name: 'phantom', type: 'lookup', reference: 'crm_nowhere_param' },
79+
],
80+
body: { language: 'js', source: 'return { ok: true };', capabilities: [] },
81+
},
82+
],
83+
},
84+
],
85+
};
86+
87+
const ARTIFACT = {
88+
manifest: { id: 'com.example.multi.core' },
89+
objects: CORE_BODY.objects,
90+
packages: [{ manifest: ORDERS_BODY }, { manifest: CORE_BODY }],
91+
} as Record<string, unknown>;
92+
93+
/**
94+
* Exactly what `compile.ts` step 3b-ii does for one entry.
95+
*
96+
* ⛔ `context` takes no DEFAULT: a default would be applied to the CONTROL
97+
* leg's explicit `undefined` — the exact value the pre-#16611 call site passes
98+
* — and that leg would silently measure the fixed shape and pass.
99+
*/
100+
const perPackageStack = (index: number, context: unknown) =>
101+
packageBodyAsStack(artifactPackages(ARTIFACT)[index].body, context);
102+
103+
describe('packageBodyAsStack — the per-package leg resolves a sibling package at BOTH reference levels (#18204)', () => {
104+
it('CONTROL — with no context the SAME body errors at both levels, so the context is what does the work', () => {
105+
// Without this leg a green assertion below is indistinguishable from a rule
106+
// that stopped judging these sites at all.
107+
const paths = validateObjectReferences(perPackageStack(0, undefined)).map((f) => f.path);
108+
expect(paths).toEqual([
109+
'objects[0].fields.account.reference',
110+
'objects[0].fields.ghost.reference',
111+
'objects[0].actions[0].params[0].reference',
112+
'objects[0].actions[0].params[1].reference',
113+
]);
114+
});
115+
116+
it('resolves the sibling\'s object at both levels, and still refuses a name no package provides', () => {
117+
const findings = validateObjectReferences(perPackageStack(0, ARTIFACT.packages));
118+
// One equality carries all four verdicts: `crm_account` is absent at BOTH
119+
// levels (resolved through `packages[]`) and the two dangling names are
120+
// present at BOTH levels (the refusal MOVED — it did not disappear).
121+
expect(findings.map((f) => f.path)).toEqual([
122+
'objects[0].fields.ghost.reference',
123+
'objects[0].actions[0].params[1].reference',
124+
]);
125+
expect(findings.map((f) => f.rule)).toEqual([
126+
'object-reference-unknown',
127+
'object-reference-unknown',
128+
]);
129+
expect(findings.map((f) => f.severity)).toEqual(['error', 'error']);
130+
});
131+
132+
it('judges the sibling from its own side unchanged', () => {
133+
// The context widens what a package can RESOLVE, never what it JUDGES: the
134+
// app package owns no reference, so it has nothing to report either way.
135+
expect(validateObjectReferences(perPackageStack(1, ARTIFACT.packages))).toEqual([]);
136+
});
137+
138+
it('hands the rule the artifact\'s own entries — the body of each package is under `manifest`', () => {
139+
// The shape is the contract between this function and the rule: the rule
140+
// reads `packages[].manifest.<collection>`, so a context reduced to some
141+
// other entry shape resolves nothing while every type still checks.
142+
const stack = perPackageStack(0, ARTIFACT.packages) as { packages?: unknown; manifest?: unknown };
143+
expect(stack.packages).toBe(ARTIFACT.packages);
144+
expect(stack.manifest).toBe(artifactPackages(ARTIFACT)[0].body);
145+
});
146+
});

‎packages/cli/test/build-multi-package-artifact.e2e.test.ts‎

Lines changed: 56 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -143,15 +143,31 @@ export default {
143143
* between the ruled fix and "skip the site per package".
144144
*
145145
* One fixture measures both, because the discriminating fact is WHICH of the
146-
* two lookups on `probe_order` is reported: `account` (a sibling provides it)
147-
* must not be, `ghost` (nothing provides it) must be. Dropping the `packages[]`
146+
* lookups on `probe_order` is reported: `account` (a sibling provides it) must
147+
* not be, `ghost` (nothing provides it) must be. Dropping the `packages[]`
148148
* pass-through reports both; dropping the judging reports neither.
149149
*
150+
* ## Two LEVELS, one artifact, one verdict (#18204)
151+
*
152+
* `probe_order` carries the same pair of names twice: once as a field
153+
* `reference` (`Field.lookup()`'s target) and once as an action param's
154+
* record-picker `reference`. Both are resolved by the same ladder in
155+
* `validate-object-references.ts` against the same set, and ADR-0130 §1.5
156+
* accepts a cross-package lookup — so a build in which one level resolves a
157+
* sibling's object and the other refuses it is the platform contradicting
158+
* itself inside a single command. That contradiction SHIPPED: `@objectstack/
159+
* cli@17.4.0` lowers a package body with `packageBodyAsStack(body)` and no
160+
* context argument at all, which refuses the action-param reference while the
161+
* field level — a site that release's rule did not yet walk — goes unjudged
162+
* and therefore looks accepted. The equality below is what keeps the two
163+
* levels from drifting apart again: it names the paths of BOTH refusals and,
164+
* by their absence, BOTH resolutions.
165+
*
150166
* ⚠️ The top level carries `probe_account`, so `objects` is PRESENT and
151167
* `authoringRuleUnionStack` folds nothing into it — it only ever fills ABSENT
152168
* keys, which `src/utils/stack-collections.test.ts` pins BY IDENTITY on exactly
153169
* this shape (a stack carrying both its collections and `packages[]`). So the
154-
* union run never sees `probe_order`'s fields, and this is a pin on
170+
* union run never sees `probe_order`'s fields or actions, and this is a pin on
155171
* `compile.ts`'s half rather than a second copy of `packages/lint`'s rule test,
156172
* whose input is a local three-key REPLICA of `packageBodyAsStack`
157173
* (`perPackageStack`) and stays green if this command stops building that
@@ -160,8 +176,9 @@ export default {
160176
* ⭐ That last claim is MEASURED rather than argued. Three ablations, each
161177
* reddening THIS case alone and leaving the other six in this file green:
162178
*
163-
* 1. drop `packages[]` from `packageBodyAsStack` (the pre-#16611 shape) — the
164-
* equality below receives BOTH paths, `account` first;
179+
* 1. drop `packages[]` from `packageBodyAsStack` (the pre-#16611 shape, and
180+
* the one `@objectstack/cli@17.4.0` ships) — the equality below receives
181+
* all FOUR paths, `account` first;
165182
* 2. hand the per-package leg no `objects` at all, i.e. the "skip the site per
166183
* package" option the ruling rejected — `os build` exits **0** with
167184
* `success: true`, so the union run is NOT a second reporter for these
@@ -185,6 +202,16 @@ const probeOrder = {
185202
account: { type: 'lookup', label: 'Account', reference: 'probe_account' },
186203
ghost: { type: 'lookup', label: 'Ghost', reference: 'probe_nothing' },
187204
},
205+
actions: [
206+
{
207+
name: 'probe_link', label: 'Link', type: 'script', locations: [],
208+
params: [
209+
{ name: 'account', label: 'Account', type: 'lookup', reference: 'probe_account' },
210+
{ name: 'phantom', label: 'Phantom', type: 'lookup', reference: 'probe_nothing_param' },
211+
],
212+
body: { language: 'js', source: 'return { ok: true };', capabilities: [] },
213+
},
214+
],
188215
};
189216
190217
export default {
@@ -289,7 +316,7 @@ describe('ADR-0130 D4 — `os build` emits one artifact carrying `packages[]`',
289316
expect(paths).toContain('packages.0.manifest.objects.0');
290317
}, 180_000);
291318

292-
it('resolves a SIBLING package\'s object on the per-package leg, and still errors on an artifact-wide dangling one', async () => {
319+
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 () => {
293320
const run = await runCli(['build', '--json'], dirs.refs);
294321
expect(run.code, `${run.stdout}\n${run.stderr}`).toBe(1);
295322
const payload = JSON.parse(run.stdout) as {
@@ -301,11 +328,29 @@ describe('ADR-0130 D4 — `os build` emits one artifact carrying `packages[]`',
301328
// is the only thing that distinguishes the two exits from outside.
302329
expect(payload.error).toBe('author-time rules failed for one or more packages');
303330
const refs = (payload.issues ?? []).filter((i) => i.rule === 'object-reference-unknown');
304-
// Both directions in one equality: `ghost` is present (the leg still
305-
// JUDGES) and `account` is absent (the leg RESOLVED it through the
306-
// artifact's `packages[]`). A pass-through that went missing reports both.
307-
expect(refs.map((i) => i.path)).toEqual(['objects[0].fields.ghost.reference']);
308-
expect(refs[0].package).toBe('com.example.probe.orders');
331+
// Every direction in ONE equality, over BOTH levels (#18204):
332+
//
333+
// present — `fields.ghost` and `params[1]` (`probe_nothing_param`): the
334+
// leg still JUDGES both levels, so the refusal MOVED rather
335+
// than disappearing when the context was added;
336+
// absent — `fields.account` and `params[0]`, both naming
337+
// `probe_account`: the leg RESOLVED that name through the
338+
// artifact's `packages[]` at the field level AND at the
339+
// action-param level, which is the agreement the two levels
340+
// owe each other.
341+
//
342+
// The order is the rule's own walk order (`validate-object-references.ts`:
343+
// object fields, then global actions, then object-embedded actions), so an
344+
// equality — not a `toContain` pair — is what keeps a silently dropped
345+
// finding visible.
346+
expect(refs.map((i) => i.path)).toEqual([
347+
'objects[0].fields.ghost.reference',
348+
'objects[0].actions[0].params[1].reference',
349+
]);
350+
expect(refs.map((i) => i.package)).toEqual([
351+
'com.example.probe.orders',
352+
'com.example.probe.orders',
353+
]);
309354
}, 180_000);
310355
});
311356

0 commit comments

Comments
 (0)