Skip to content

Commit 6783718

Browse files
committed
test(cli): the #18490 package-id pins move to the post-narrowing reality
CI on this branch's merge head reds `packages/cli`'s integration tier: the #18490 file composes an artifact whose `manifest.id` is `''` and parses it through `ObjectStackDefinitionSchema`, which this branch's pattern now refuses. That file said it would red first and that the pins under it would then measure nothing, so the red is converted into the pin it asked for: the first case now pins the refusal itself, on `manifest.id`, with a lit control proving the id is what decides it, and the three cross-door cases keep measuring that the build names a package exactly as the runtime fold does at an identity a command can actually hand down. The divergence the file was written around is structurally closed — a parsed package's id is non-empty by construction, so the owner's `id || name` never falls back. The degenerate fixture is not kept alive by bypassing the parse. Claude-Session: https://claude.ai/code/session_012GcsUbuqFGBibkEDMRC1eE Co-authored-by: Claude <noreply@anthropic.com>
1 parent 0fc32b4 commit 6783718

1 file changed

Lines changed: 72 additions & 25 deletions

File tree

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

Lines changed: 72 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,26 @@
2727
* `ObjectQL`, and the assertion is that the two STRINGS match — so the day
2828
* either rule moves, this reds instead of agreeing with itself.
2929
*
30+
* ## ⭐ #17534 — the divergent input is no longer reachable through the parse
31+
*
32+
* This file's first case used to be a floor: `ManifestSchema` constrained
33+
* NEITHER id key to be non-empty, so `''` parsed and the two rules could
34+
* disagree on a stack `os build` would accept. It said, in as many words, that
35+
* a spec change refusing that input should red HERE first. #17534 is that
36+
* change: `ManifestSchema.id` carries `MANIFEST_ID_PATTERN`, so a composed
37+
* artifact whose `manifest.id` is `''` is refused at the parse door, by name.
38+
*
39+
* What that does to this file is structural, not cosmetic — the owner derives
40+
* `manifest.id || manifest.name`, and after #17534 a parsed package's `id` is
41+
* always a pattern-matching, non-empty string, so the fallback never runs and
42+
* the two rules can no longer disagree on ANY parsed input. So the first case
43+
* is now the closure itself (the refusal, quoted), and the cross-door cases
44+
* keep measuring what they were written to measure — that the build names a
45+
* package EXACTLY as the runtime fold does, and never with the deleted copy's
46+
* positional spelling — at an identity a command can actually hand down.
47+
* ⛔ The degenerate fixture is not kept alive by bypassing the parse: a pin on
48+
* an input no command can produce measures nothing and reads as if it did.
49+
*
3050
* ## Why this is a file of its own, and why it is INTEGRATION tier
3151
*
3252
* The cross-door half constructs `new ObjectQL(`, which is a KERNEL signal in
@@ -50,6 +70,8 @@ const APP = 'multi_crm';
5070
const GROUP = 'sales_group';
5171
const TYPO = 'sales_grp';
5272
const CORE_ID = 'com.example.multi.core';
73+
/** The contributing package's id — reverse-domain, as #17534 requires of every parsed manifest. */
74+
const ORDERS_ID = 'com.example.multi.orders';
5375

5476
/** The App package — owns the app and the group container the module aims at. */
5577
const coreStack = () => ({
@@ -79,17 +101,22 @@ const coreStack = () => ({
79101
});
80102

81103
/**
82-
* The contributing package, carrying the divergent identity: both id keys `''`.
104+
* The contributing package.
83105
*
84106
* ⚠️ Its own namespace, not the app package's. The composed-artifact leg does
85107
* not care, but the runtime leg INSTALLS both packages into one registry and
86108
* ADR-0048 refuses a second package under a namespace another already owns —
87109
* a conflict that would make this file red for a reason that has nothing to do
88110
* with what it measures.
111+
*
112+
* ⭐ #17534: `id` is the identity both doors derive; `name` stays `''` so the
113+
* owner's `id || name` is still exercised on a manifest that gives the fallback
114+
* nothing to fall back TO — the nearest reachable neighbour of the input this
115+
* file was written around.
89116
*/
90-
const emptyIdOrdersStack = (group: string) => ({
117+
const ordersStack = (group: string, id: string = ORDERS_ID) => ({
91118
manifest: {
92-
id: '',
119+
id,
93120
name: '',
94121
namespace: 'ord',
95122
version: '1.0.0',
@@ -108,35 +135,51 @@ const emptyIdOrdersStack = (group: string) => ({
108135
}],
109136
});
110137

111-
/** The artifact as the commands actually hand it down — through their own parse. */
112-
const parsedEmptyIdArtifact = (group: string): AnyRec => {
138+
/** The composed artifact, up to but NOT through the parse — the parse is what two cases below read. */
139+
const composedArtifact = (group: string, id: string = ORDERS_ID): AnyRec => {
113140
const composed = composeStacks(
114-
[emptyIdOrdersStack(group), coreStack()],
141+
[ordersStack(group, id), coreStack()],
115142
{ manifest: 'preserve' },
116143
) as unknown as Record<string, unknown>;
117-
const normalized = normalizeStackInput(composed, { onConversionNotice: () => {} });
118-
const result = ObjectStackDefinitionSchema.safeParse(normalized);
144+
return normalizeStackInput(composed, { onConversionNotice: () => {} }) as unknown as AnyRec;
145+
};
146+
147+
/** The artifact as the commands actually hand it down — through their own parse. */
148+
const parsedArtifact = (group: string, id: string = ORDERS_ID): AnyRec => {
149+
const result = ObjectStackDefinitionSchema.safeParse(composedArtifact(group, id));
119150
if (!result.success) {
120151
throw new Error(`fixture does not parse: ${JSON.stringify(result.error.issues.slice(0, 3))}`);
121152
}
122153
return result.data as unknown as AnyRec;
123154
};
124155

125156
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]);
157+
it('⭐ #17534 — an empty `manifest.id` no longer REACHES this check: the parse door refuses it by name', () => {
158+
// The floor, inverted. It used to read: `ManifestSchema` constrains NEITHER
159+
// id key to be non-empty, so `''` parses and the two rules can disagree on
160+
// a stack `os build` would accept — and it promised to red FIRST if a spec
161+
// change started refusing that input. This is that red, converted into the
162+
// pin it asked for: `ManifestSchema.id` carries `MANIFEST_ID_PATTERN`, so
163+
// the composed artifact is refused at the parse, on `manifest.id`, with the
164+
// value echoed. ⇒ Every parsed package now has a non-empty id, the owner's
165+
// `id || name` never falls back, and the divergence this file was written
166+
// around cannot occur on any input a command hands down.
167+
const refused = ObjectStackDefinitionSchema.safeParse(composedArtifact(GROUP, ''));
168+
expect(refused.success).toBe(false);
169+
const issues = refused.success ? [] : refused.error.issues;
170+
expect(issues.some((issue) => issue.path.join('.') === 'packages.0.manifest.id')).toBe(true);
171+
expect(issues.map((issue) => issue.message).join('\n')).toContain("Invalid package id ''");
172+
173+
// Lit control — the id is what decides it: the same fixture with a
174+
// reverse-domain id parses, and the owner names both packages.
175+
expect(artifactPackages(parsedArtifact(GROUP)).map((pkg) => pkg.id).sort())
176+
.toEqual([CORE_ID, ORDERS_ID]);
134177
});
135178

136179
it('the build names that package EXACTLY as the runtime fold does', async () => {
137180
// The build door: the shipped derivation, reached the way both commands
138181
// reach it — `findNavGroupDiagnostics(result.data)`, one argument.
139-
const built = await findNavGroupDiagnostics(parsedEmptyIdArtifact(TYPO));
182+
const built = await findNavGroupDiagnostics(parsedArtifact(TYPO));
140183
expect(built).toHaveLength(1);
141184

142185
// The runtime door: the same two packages installed into a real registry.
@@ -146,7 +189,7 @@ describe('#18490 — one package, two doors, one name', () => {
146189
const engine = new ObjectQL();
147190
const core = coreStack();
148191
engine.registerApp({ ...core.manifest, apps: core.apps });
149-
engine.registerApp({ ...emptyIdOrdersStack(TYPO).manifest });
192+
engine.registerApp({ ...ordersStack(TYPO).manifest });
150193
engine.registry.getApp(APP);
151194
const folded = engine.registry.getAppNavDiagnostics(APP);
152195
expect(folded).toHaveLength(1);
@@ -161,20 +204,24 @@ describe('#18490 — one package, two doors, one name', () => {
161204
// Its own assertion, because the pin above would also pass if BOTH doors
162205
// moved to `packages[0]`. The runtime has no positional fallback, so a
163206
// 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('');
207+
const built = await findNavGroupDiagnostics(parsedArtifact(TYPO));
208+
expect(built[0].packageId).toBe(ORDERS_ID);
166209
expect(built[0].packageId).not.toBe('packages[0]');
167210
expect(built[0].message).not.toContain('packages[0]');
168211
});
169212

170213
it('two packages that both resolve to the empty id still produce TWO findings', async () => {
171214
// The id is CARRIED and PRINTED on this path — never a map key, a dedupe
172215
// 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.
216+
// failure a SHARED id would cause if it ever became one, and it would
217+
// present as the report going QUIET rather than as an error. ⭐ #17534 moved
218+
// the shared value from `''` to a reverse-domain id; the property under
219+
// test — two packages resolving to ONE name still produce two findings — is
220+
// unchanged, and a duplicate id is still reachable here because this path
221+
// carries the id rather than keying on it.
175222
const second = {
176223
manifest: {
177-
id: '',
224+
id: ORDERS_ID,
178225
name: '',
179226
navigationContributions: [{
180227
app: APP,
@@ -183,13 +230,13 @@ describe('#18490 — one package, two doors, one name', () => {
183230
}],
184231
},
185232
};
186-
const parsed = parsedEmptyIdArtifact(TYPO);
233+
const parsed = parsedArtifact(TYPO);
187234
const widened: AnyRec = {
188235
...parsed,
189236
packages: [...((parsed.packages ?? []) as unknown[]), second],
190237
};
191238
const found = await findNavGroupDiagnostics(widened);
192239
expect(found).toHaveLength(2);
193-
expect(found.map((d) => d.packageId)).toEqual(['', '']);
240+
expect(found.map((d) => d.packageId)).toEqual([ORDERS_ID, ORDERS_ID]);
194241
});
195242
});

0 commit comments

Comments
 (0)