Skip to content

Commit 6a886b8

Browse files
committed
wip: split the cross-door package-id pin into its own integration-tier file
Claude-Session: https://claude.ai/code/session_01DvvamiacK328idtBYJBxV3 Co-authored-by: Claude <noreply@anthropic.com>
1 parent cc4144a commit 6a886b8

3 files changed

Lines changed: 212 additions & 134 deletions

File tree

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
---
2+
"@objectstack/cli": patch
3+
---
4+
5+
`os build` / `os validate` name an artifact package the same way the runtime fold does when its `manifest.id` and `manifest.name` are both empty — `nav-contribution-groups.ts` no longer carries its own copy of the artifact package-id rule and imports the declared owner instead (#18490).
6+
7+
Clause-②: no
8+
9+
`packages/cli/src/utils/artifact-packages.ts` declares itself the sole owner of "which package is this", and says why in its own header: *"⛔ A second copy is the one that must not happen. … Two readers computing 'which package is this' slightly differently is how one entry comes to judge a different set of packages than the other while both look right."* `nav-contribution-groups.ts` exported a second implementation, `artifactPackagesOf`, which differed from the owner in one guard — a non-empty check on `manifest.name` — and the two had already drifted on a real input.
10+
11+
- **The divergent input is reachable, measured rather than assumed.** `ManifestSchema` requires `id` and `name` as strings and constrains neither to be non-empty, so `{ manifest: { id: '', name: '', … } }` parses green through the same `normalizeStackInput` + `ObjectStackDefinitionSchema` chain both commands run. For that package the owner answered `''` and the deleted copy answered `` `packages[<index>]` ``.
12+
- **Importing the owner chose `''`, and `''` is the answer this path needs.** `ObjectQL.registerApp` derives the id it registers a navigation contribution under as `manifest.id || manifest.name`, with no positional fallback, so the read-time fold names that package `''` and prints `Package "" contributes …`. The build used to print `Package "packages[0]" …` for the same artifact — two doors naming one package differently, which is the divergence the shared `checkNavContributionGroups` predicate exists to prevent, one field over.
13+
- **What an author sees change**: for an artifact package with an empty `id` *and* an empty `name`, the `packageId` on a `nav_contribution_group_missing` warning — and the package name inside its message — is now `''` instead of `packages[<index>]`, in both `os build` and `os validate`, matching what the runtime already reports at boot. Every package with a non-empty `id` or `name` is unaffected: both rules answered identically there, measured on the control legs.
14+
- **The id is carried and printed, never keyed on.** Two packages that both resolve to `''` still produce two findings rather than collapsing into one — pinned, because that failure mode would present as a report going quiet rather than as an error.
15+
16+
`artifactPackagesOf` is removed. It was never reachable through this package's `exports` map (`.`, `./console`, `./hook-body`), so no consumer import can break; the removal is internal to `dist`.
Lines changed: 195 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,195 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* #18490 — the artifact package-id rule has ONE owner again, and importing it
5+
* DECIDED the one input the two copies disagreed on.
6+
*
7+
* ## What this file measures, and against what
8+
*
9+
* `packages/cli/src/utils/artifact-packages.ts` declares itself the sole owner
10+
* of "which package is this". `nav-contribution-groups.ts` carried a second
11+
* implementation, `artifactPackagesOf`, differing from the owner in a single
12+
* guard — a non-empty check on `manifest.name`. For a package whose
13+
* `manifest.id` AND `manifest.name` are both `''` the owner answered `''` and
14+
* the copy answered `packages[<index>]`.
15+
*
16+
* Deleting the copy chose `''`. What this file pins is not that string but the
17+
* REASON it is the right one: `''` is what the RUNTIME fold names that package,
18+
* and `packages[<index>]` is a spelling the runtime cannot produce at all
19+
* (`ObjectQL.registerApp` derives `manifest.id || manifest.name`, with no
20+
* positional fallback). `nav-contribution-groups.ts`'s own header says the id
21+
* it carries is "the string the runtime registers a contribution under, so a
22+
* command names a package the same way the fold does" — this is that sentence,
23+
* measured.
24+
*
25+
* ⛔ NEITHER side's rule is re-spelled here. The build's answer comes out of
26+
* the shipped `findNavGroupDiagnostics`, the runtime's out of a real
27+
* `ObjectQL`, and the assertion is that the two STRINGS match — so the day
28+
* either rule moves, this reds instead of agreeing with itself.
29+
*
30+
* ## Why this is a file of its own, and why it is INTEGRATION tier
31+
*
32+
* The cross-door half constructs `new ObjectQL(`, which is a KERNEL signal in
33+
* `vitest-tiers.ts`'s predicate — so a file carrying it is integration tier by
34+
* derivation, not by naming. Folding these cases into the unit-tier
35+
* `nav-contribution-groups.test.ts` would have moved that whole file, and its
36+
* nine existing #14553 pins with it, out of the tier they were written for.
37+
* Splitting keeps the tier change confined to the cases that actually boot a
38+
* registry. ⛔ Do not merge this file back into that one.
39+
*/
40+
41+
import { describe, it, expect } from 'vitest';
42+
import { composeStacks, normalizeStackInput, ObjectStackDefinitionSchema } from '@objectstack/spec';
43+
import { ObjectQL } from '@objectstack/objectql/core';
44+
import { findNavGroupDiagnostics } from './nav-contribution-groups.js';
45+
import { artifactPackages } from './artifact-packages.js';
46+
47+
type AnyRec = Record<string, unknown>;
48+
49+
const APP = 'multi_crm';
50+
const GROUP = 'sales_group';
51+
const TYPO = 'sales_grp';
52+
const CORE_ID = 'com.example.multi.core';
53+
54+
/** The App package — owns the app and the group container the module aims at. */
55+
const coreStack = () => ({
56+
manifest: {
57+
id: CORE_ID,
58+
name: 'Multi-Package Core',
59+
namespace: 'crm',
60+
version: '1.0.0',
61+
type: 'app' as const,
62+
},
63+
objects: [{
64+
name: 'crm_account',
65+
label: 'Account',
66+
sharingModel: 'private' as const,
67+
fields: { name: { name: 'name', type: 'text' as const, label: 'Account Name', required: true } },
68+
}],
69+
apps: [{
70+
name: APP,
71+
label: 'Multi-Package CRM',
72+
navigation: [{
73+
id: GROUP,
74+
type: 'group' as const,
75+
label: 'Sales',
76+
children: [{ id: 'nav_accounts', type: 'object' as const, objectName: 'crm_account', label: 'Accounts' }],
77+
}],
78+
}],
79+
});
80+
81+
/**
82+
* The contributing package, carrying the divergent identity: both id keys `''`.
83+
*
84+
* ⚠️ Its own namespace, not the app package's. The composed-artifact leg does
85+
* not care, but the runtime leg INSTALLS both packages into one registry and
86+
* ADR-0048 refuses a second package under a namespace another already owns —
87+
* a conflict that would make this file red for a reason that has nothing to do
88+
* with what it measures.
89+
*/
90+
const emptyIdOrdersStack = (group: string) => ({
91+
manifest: {
92+
id: '',
93+
name: '',
94+
namespace: 'ord',
95+
version: '1.0.0',
96+
type: 'module' as const,
97+
navigationContributions: [{
98+
app: APP,
99+
group,
100+
items: [{ id: 'nav_orders', type: 'object' as const, objectName: 'ord_order', label: 'Orders' }],
101+
}],
102+
},
103+
objects: [{
104+
name: 'ord_order',
105+
label: 'Order',
106+
sharingModel: 'private' as const,
107+
fields: { name: { name: 'name', type: 'text' as const, label: 'Order Number', required: true } },
108+
}],
109+
});
110+
111+
/** The artifact as the commands actually hand it down — through their own parse. */
112+
const parsedEmptyIdArtifact = (group: string): AnyRec => {
113+
const composed = composeStacks(
114+
[emptyIdOrdersStack(group), coreStack()],
115+
{ manifest: 'preserve' },
116+
) as unknown as Record<string, unknown>;
117+
const normalized = normalizeStackInput(composed, { onConversionNotice: () => {} });
118+
const result = ObjectStackDefinitionSchema.safeParse(normalized);
119+
if (!result.success) {
120+
throw new Error(`fixture does not parse: ${JSON.stringify(result.error.issues.slice(0, 3))}`);
121+
}
122+
return result.data as unknown as AnyRec;
123+
};
124+
125+
describe('#18490 — one package, two doors, one name', () => {
126+
it('an empty `manifest.id` AND `manifest.name` REACHES this check — the divergence is not hypothetical', () => {
127+
// The floor under everything below. `ManifestSchema` requires both keys as
128+
// strings and constrains NEITHER to be non-empty, so `''` parses — which is
129+
// the only reason the two rules could ever disagree on a stack `os build`
130+
// or `os validate` would actually accept. If a spec change starts refusing
131+
// it, this reds FIRST and says the pins under it now measure nothing.
132+
const parsed = parsedEmptyIdArtifact(GROUP);
133+
expect(artifactPackages(parsed).map((pkg) => pkg.id).sort()).toEqual(['', CORE_ID]);
134+
});
135+
136+
it('the build names that package EXACTLY as the runtime fold does', async () => {
137+
// The build door: the shipped derivation, reached the way both commands
138+
// reach it — `findNavGroupDiagnostics(result.data)`, one argument.
139+
const built = await findNavGroupDiagnostics(parsedEmptyIdArtifact(TYPO));
140+
expect(built).toHaveLength(1);
141+
142+
// The runtime door: the same two packages installed into a real registry.
143+
// `registerApp` derives the id it registers the contribution under, the
144+
// read-time fold relocates the mis-aimed items, and recording that
145+
// relocation is what produces the diagnostic.
146+
const engine = new ObjectQL();
147+
const core = coreStack();
148+
engine.registerApp({ ...core.manifest, apps: core.apps });
149+
engine.registerApp({ ...emptyIdOrdersStack(TYPO).manifest });
150+
engine.registry.getApp(APP);
151+
const folded = engine.registry.getAppNavDiagnostics(APP);
152+
expect(folded).toHaveLength(1);
153+
154+
// The card, in two lines: the `packageId` field a consumer reads, and the
155+
// sentence an author reads.
156+
expect(built[0].packageId).toBe(folded[0].packageId);
157+
expect(built[0].message).toBe(folded[0].message);
158+
});
159+
160+
it('⛔ and that shared name is NOT the deleted copy\'s positional spelling', async () => {
161+
// Its own assertion, because the pin above would also pass if BOTH doors
162+
// moved to `packages[0]`. The runtime has no positional fallback, so a
163+
// build printing one is a build naming a package the runtime never will.
164+
const built = await findNavGroupDiagnostics(parsedEmptyIdArtifact(TYPO));
165+
expect(built[0].packageId).toBe('');
166+
expect(built[0].packageId).not.toBe('packages[0]');
167+
expect(built[0].message).not.toContain('packages[0]');
168+
});
169+
170+
it('two packages that both resolve to the empty id still produce TWO findings', async () => {
171+
// The id is CARRIED and PRINTED on this path — never a map key, a dedupe
172+
// key or a sort key. Pinned because "both collapse into one finding" is the
173+
// failure an empty id would cause if it ever became one, and it would
174+
// present as the report going QUIET rather than as an error.
175+
const second = {
176+
manifest: {
177+
id: '',
178+
name: '',
179+
navigationContributions: [{
180+
app: APP,
181+
group: TYPO,
182+
items: [{ id: 'nav_second', type: 'object' as const, objectName: 'ord_order', label: 'Second' }],
183+
}],
184+
},
185+
};
186+
const parsed = parsedEmptyIdArtifact(TYPO);
187+
const widened: AnyRec = {
188+
...parsed,
189+
packages: [...((parsed.packages ?? []) as unknown[]), second],
190+
};
191+
const found = await findNavGroupDiagnostics(widened);
192+
expect(found).toHaveLength(2);
193+
expect(found.map((d) => d.packageId)).toEqual(['', '']);
194+
});
195+
});

‎packages/cli/src/utils/nav-contribution-groups.test.ts‎

Lines changed: 1 addition & 134 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@
5151
// The production import stays lazy and stays where it is: this line decides
5252
// only WHERE the first load is paid in THIS suite, and `os build`'s cold path
5353
// must not pull the data engine in to judge a stack with no contributions.
54-
import { ObjectQL } from '@objectstack/objectql/core';
54+
import '@objectstack/objectql/core';
5555
import { describe, it, expect } from 'vitest';
5656
import { composeStacks, normalizeStackInput, ObjectStackDefinitionSchema } from '@objectstack/spec';
5757
import { artifactPackages } from './artifact-packages.js';
@@ -293,136 +293,3 @@ describe('#14553 — `os build` checks `navigationContributions[].group` across
293293
expect(found[0].group).toBe('nav_accounts');
294294
});
295295
});
296-
297-
/**
298-
* #18490 — the package-id rule has ONE owner again, and importing it decided
299-
* the one input the two copies disagreed on.
300-
*
301-
* `nav-contribution-groups.ts` used to carry its own `artifactPackagesOf`. It
302-
* differed from `artifactPackages` in a single guard: a non-empty check on
303-
* `manifest.name`. For a package whose `manifest.id` AND `manifest.name` are
304-
* both `''` the owner answered `''` and the copy answered `packages[<index>]`.
305-
* Deleting the copy chose `''`, so what this block pins is not the string but
306-
* the REASON it is right: `''` is what the runtime fold names that package,
307-
* and `packages[<index>]` is a spelling the runtime cannot produce at all.
308-
*
309-
* ⛔ Neither side's rule is re-spelled here. The build's answer comes out of
310-
* the shipped `findNavGroupDiagnostics`, the runtime's out of a real
311-
* `ObjectQL`, and the assertion is that the two STRINGS match — so the day
312-
* either rule moves, this reds instead of agreeing with itself.
313-
*
314-
* `ObjectQL` is taken from `@objectstack/objectql/core`, the entry this file
315-
* already loads at module top (see the note there): the pin adds assertions,
316-
* not a module graph.
317-
*/
318-
describe('#18490 — the artifact package-id rule is the owner\'s, and both doors name one package alike', () => {
319-
const EMPTY_PKG_NAMESPACE = 'ord';
320-
321-
/** The contributing package, with the divergent identity: both keys `''`. */
322-
const emptyIdOrdersStack = (group: string) => ({
323-
manifest: {
324-
id: '',
325-
name: '',
326-
namespace: EMPTY_PKG_NAMESPACE,
327-
version: '1.0.0',
328-
type: 'module' as const,
329-
navigationContributions: [{
330-
app: APP,
331-
group,
332-
items: [{ id: 'nav_orders', type: 'object' as const, objectName: 'ord_order', label: 'Orders' }],
333-
}],
334-
},
335-
objects: [{
336-
name: 'ord_order',
337-
label: 'Order',
338-
sharingModel: 'private' as const,
339-
fields: { name: { name: 'name', type: 'text' as const, label: 'Order Number', required: true } },
340-
}],
341-
});
342-
343-
/** Through the SAME parse chain `compile.ts` and `validate.ts` run. */
344-
const parsedEmptyIdArtifact = (group: string): AnyRec => {
345-
const composed = composeStacks(
346-
[emptyIdOrdersStack(group), coreStack()],
347-
{ manifest: 'preserve' },
348-
) as unknown as Record<string, unknown>;
349-
const normalized = normalizeStackInput(composed, { onConversionNotice: () => {} });
350-
const result = ObjectStackDefinitionSchema.safeParse(normalized);
351-
if (!result.success) {
352-
throw new Error(`fixture does not parse: ${JSON.stringify(result.error.issues.slice(0, 3))}`);
353-
}
354-
return result.data as unknown as AnyRec;
355-
};
356-
357-
it('an empty `manifest.id` AND `manifest.name` REACHES this check — the divergence is not hypothetical', () => {
358-
// The floor under the two assertions below. `ManifestSchema` requires both
359-
// keys as strings and constrains neither to be non-empty, so `''` parses —
360-
// which is the only reason the two rules' disagreement was ever reachable
361-
// from `os build` / `os validate` at all. If a future spec change refuses
362-
// it, this reds FIRST and says the pins under it now measure nothing.
363-
const parsed = parsedEmptyIdArtifact(GROUP);
364-
const ids = packagesOf(parsed).map((pkg) => pkg.id).sort();
365-
expect(ids).toEqual(['', CORE_ID]);
366-
});
367-
368-
it('the build names that package EXACTLY as the runtime fold does', async () => {
369-
const parsed = parsedEmptyIdArtifact('sales_grp');
370-
const built = await findNavGroupDiagnostics(parsed);
371-
expect(built).toHaveLength(1);
372-
373-
// The same artifact installed into a real registry: `registerApp` derives
374-
// the id it registers a contribution under, `getApp` runs the fold, and
375-
// the fold records its own diagnostic for the relocation.
376-
const engine = new ObjectQL();
377-
const core = coreStack();
378-
engine.registerApp({ ...core.manifest, apps: core.apps });
379-
engine.registerApp({ ...emptyIdOrdersStack('sales_grp').manifest });
380-
engine.registry.getApp(APP);
381-
const folded = engine.registry.getAppNavDiagnostics(APP);
382-
expect(folded).toHaveLength(1);
383-
384-
// The whole point of the card, in two lines: one package, two doors, one
385-
// name — the `packageId` field a consumer reads and the sentence an author
386-
// reads.
387-
expect(built[0].packageId).toBe(folded[0].packageId);
388-
expect(built[0].message).toBe(folded[0].message);
389-
});
390-
391-
it('⛔ and that shared name is NOT the deleted copy\'s positional spelling', async () => {
392-
// Stated as its own assertion because the pin above would also pass if
393-
// BOTH doors moved to `packages[0]`. The runtime has no positional
394-
// fallback (`manifest.id || manifest.name`), so a build printing one is a
395-
// build naming a package the runtime never will.
396-
const parsed = parsedEmptyIdArtifact('sales_grp');
397-
const built = await findNavGroupDiagnostics(parsed);
398-
expect(built[0].packageId).toBe('');
399-
expect(built[0].packageId).not.toBe('packages[0]');
400-
expect(built[0].message).not.toContain('packages[0]');
401-
});
402-
403-
it('two packages that both resolve to the empty id still produce TWO findings', async () => {
404-
// The id is carried and printed, never used as a map key, a dedupe key or
405-
// a sort key on this path. Pinned because "both collapse to one finding"
406-
// is the failure mode an empty id would have if it ever became one, and it
407-
// would present as a REPORT GOING QUIET rather than as an error.
408-
const second = {
409-
manifest: {
410-
id: '',
411-
name: '',
412-
navigationContributions: [{
413-
app: APP,
414-
group: 'sales_grp',
415-
items: [{ id: 'nav_second', type: 'object' as const, objectName: 'ord_order', label: 'Second' }],
416-
}],
417-
},
418-
};
419-
const parsed = parsedEmptyIdArtifact('sales_grp');
420-
const widened: AnyRec = {
421-
...parsed,
422-
packages: [...((parsed.packages ?? []) as unknown[]), second],
423-
};
424-
const found = await findNavGroupDiagnostics(widened);
425-
expect(found).toHaveLength(2);
426-
expect(found.map((d) => d.packageId)).toEqual(['', '']);
427-
});
428-
});

0 commit comments

Comments
 (0)