Skip to content

Commit b63b526

Browse files
committed
fix(spec): guard the artifact cross-reference pass and declare what it narrows
The artifact pass added for #18202 ran the two artifact-scoped collectors over every composed input, including inputs the strict parse never saw. Three corrections, all inside this PR's own surface: - `collectSeedDataObjectErrors` and `collectPermissionGrantObjectErrors` now guard their own shapes the way `composeStacks`'s step-3 concat pass does. A non-array `permissions` / `data` is announced through `warnMalformedCollectionKey` (deduplicated per key, so the two passes speak once) and skipped; a non-object entry, and a non-string `object`, carry no reference for the rule to resolve and are skipped. Before this, an unparsed input that `origin/main` composes with a warning raised a bare `TypeError` with no `code` and no `status` — a crash outside the ADR-0112 envelope, in the file whose refusal discipline is that envelope. - The compatibility claim is replaced by the invariant that actually holds. An input that passed the strict `defineStack` parse cannot newly fail at composition; an input that BYPASSED it (`strict: false`, a hand-built stack object) is checked for these two rules at composition for the first time and a dangling reference in it is now refused. That narrowing is declared in the docstring, the changeset and the fixtures instead of being claimed away. - `composeStacks` returns `stacks[0]` untouched, so the artifact pass never runs for a single input. The option docstring and the docs bullet now say "in a composition of two or more packages" rather than stating the guarantee unqualified. Fixtures pin all three: the refusals on unparsed inputs, the malformed-shape guards, and the one-input boundary with its two-package contrast. Claude-Session: https://claude.ai/code/session_01T3YsvpK1PvYf9n1YUhYP6W Co-authored-by: Claude <noreply@anthropic.com>
1 parent b007237 commit b63b526

4 files changed

Lines changed: 303 additions & 30 deletions

File tree

‎.changeset/artifact-scoped-cross-reference.md‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@ const app = defineStack(appConfig, { // owns crm_account,
1616
export default composeStacks([service, app], { manifest: 'preserve' });
1717
```
1818

19-
Nothing else widens. `hooks[].object` and an app's own `navigation` `objectName` stay refused against the stack's own objects even when the name is listed, because ADR-0130 §1.5 records both refusals as the shape of the package seam. A stack that does not pass `artifactObjects` — every single-package app — validates exactly as before.
19+
Nothing else widens. `hooks[].object` and an app's own `navigation` `objectName` stay refused against the stack's own objects even when the name is listed, because ADR-0130 §1.5 records both refusals as the shape of the package seam.
2020

21-
The refusal moved rather than disappearing: `composeStacks` now re-checks those two classes over the composed artifact, so a name `artifactObjects` claims and no package in the artifact defines is refused there, with the same `STACK_CROSS_REFERENCE_INVALID` code, the same `422`, and the same per-finding message. Only the header differs, naming the pass that refused it.
21+
The refusal moved rather than disappearing: in a composition of **two or more** packages, `composeStacks` now re-checks those two classes over the composed artifact, so a name `artifactObjects` claims and no package in the artifact defines is refused there, with the same `STACK_CROSS_REFERENCE_INVALID` code, the same `422`, and the same per-finding message. Only the header differs, naming the pass that refused it. `composeStacks` returns a single input untouched, so a one-package composition does not re-check the claim.
22+
23+
**What that changes about which inputs `composeStacks` accepts.** `defineStack` itself is unchanged for a stack that does not pass `artifactObjects` — every single-package app validates exactly as before. `composeStacks` is not: it applies the two artifact-scoped rules to **every** input carrying objects, not only the ones that opted in. For an input that passed the strict `defineStack` parse that is a no-op, so such an input cannot newly fail. For an input that **bypassed** the strict parse it is not: `defineStack(config, { strict: false })` returns before cross-reference validation runs, and a hand-built stack object never enters it, so these two rules have never been applied to it. Such an input carrying a dangling `permissions[].objects` key or `data[].object` is now refused at composition where it previously composed with only a warning. If you compose unparsed stacks, that is the one behavioural change to expect; a malformed `permissions` / `data` on such an input is still skipped with the existing non-array warning rather than raising.

‎content/docs/getting-started/examples.mdx‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -438,9 +438,12 @@ The rules worth knowing before you split a product this way:
438438
artifact — and seed rows are the same shape. Both resolve against the artifact rather than the one
439439
stack, but only once you hand `defineStack` the sibling object names:
440440
`defineStack(appConfig, { artifactObjects: ordersStack.objects?.map((o) => o.name) })`.
441-
`composeStacks` then refuses any name no package in the artifact defines, so a
442-
typo is still caught — one pass later, and by name. Hooks are the deliberate
443-
exception: a hook still belongs to the package that owns its object.
441+
**In a composition of two or more packages**, `composeStacks` then refuses any
442+
name no package in the artifact defines, so a typo is still caught — one pass
443+
later, and by name. The qualifier is real: `composeStacks` returns a single
444+
stack untouched, so composing one package on its own does not check the claim.
445+
Hooks are the deliberate exception: a hook still belongs to the package that
446+
owns its object.
444447
- **One artifact, one version.** Everything inside ships, installs and upgrades
445448
together; you cannot hot-fix one module on its own. A module that needs its
446449
own release cadence belongs in its own artifact.

‎packages/spec/src/stack-artifact-crossref.test.ts‎

Lines changed: 199 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,26 @@
3939
* because only the pass differs. `ArtifactPass` is that fixture, and it is an
4040
* acceptance criterion of #18202 rather than a nicety.
4141
*
42+
* ## What the artifact pass DOES newly refuse, and what it must never do
43+
*
44+
* The compatibility statement that holds is not "nothing can newly fail". It
45+
* is two statements:
46+
*
47+
* - an input that passed the strict `defineStack` parse cannot newly fail at
48+
* composition — its references already resolved against its own objects, a
49+
* subset of the composed set;
50+
* - an input that BYPASSED the strict parse (`strict: false`, a hand-built
51+
* stack object) is checked for these two rules at composition for the FIRST
52+
* time, and a dangling reference in it is refused where it previously
53+
* composed.
54+
*
55+
* The second is a narrowing, it is deliberate, and the three blocks at the
56+
* bottom of this file pin it as declared behaviour: the refusals themselves,
57+
* the shape guards that keep an unparsed malformed collection a warning rather
58+
* than a bare `TypeError` outside the ADR-0112 envelope, and the one-input
59+
* boundary that makes "in a composition of two or more packages" the only
60+
* correct way to state the guarantee to an author.
61+
*
4262
* ## Fixture shape
4363
*
4464
* The two-package shape measured downstream, reproduced at its smallest: the
@@ -295,3 +315,182 @@ describe('#18202 — the object-less leniency the ARTIFACT pass inherits verbati
295315
.toBeNull();
296316
});
297317
});
318+
319+
/**
320+
* The ARTIFACT pass runs over EVERY input, not only the ones that opted in —
321+
* and for an input that never went through the strict `defineStack` parse that
322+
* is not a no-op. `defineStack(config, { strict: false })` returns before
323+
* `validateCrossReferences` runs at all, and a hand-built stack object never
324+
* enters it, so these two rules have never been applied to such an input. The
325+
* artifact pass is the first place they are.
326+
*
327+
* That is a real narrowing of what `composeStacks` accepts, in the Prime
328+
* Directive #12 direction. It is pinned here as DECLARED behaviour so the next
329+
* reader meets it as a decision rather than as a regression: the changeset says
330+
* it, `collectArtifactCrossReferenceErrors`'s docstring says it, and these
331+
* fixtures hold it.
332+
*/
333+
describe('#18202 — an input that bypassed the strict parse IS checked at composition', () => {
334+
/** The app package as an unparsed stack: `strict: false` skips every validation. */
335+
const unparsedApp = (grantObject: string, seedObject: string) =>
336+
defineStack(appConfig(grantObject, seedObject), { strict: false });
337+
338+
/** The same config as a hand-built object — it never enters `defineStack` at all. */
339+
const handBuiltApp = (grantObject: string, seedObject: string) =>
340+
appConfig(grantObject, seedObject) as unknown as ReturnType<typeof defineStack>;
341+
342+
it('`strict: false` alone still composes — the parse it skipped is not reinstated here', () => {
343+
// The control for the two refusals below: same construction, a name the
344+
// artifact DOES define. If this went red the refusals would prove nothing.
345+
expect(
346+
refusalOf(() => composeStacks([serviceStack(), unparsedApp('crm_case', 'crm_case')], { manifest: 'preserve' })),
347+
).toBeNull();
348+
});
349+
350+
it('REFUSES a `strict: false` input whose grant names an object NO package defines', () => {
351+
const refused = refusalOf(() =>
352+
composeStacks([serviceStack(), unparsedApp(NOWHERE, 'crm_case')], { manifest: 'preserve' }),
353+
);
354+
expect(refused?.code).toBe('STACK_CROSS_REFERENCE_INVALID');
355+
expect(refused?.status).toBe(422);
356+
expect(refused?.issues).toContain(GRANT_ON_NOWHERE);
357+
});
358+
359+
it('REFUSES the seed-data twin on a `strict: false` input', () => {
360+
const refused = refusalOf(() =>
361+
composeStacks([serviceStack(), unparsedApp('crm_case', NOWHERE)], { manifest: 'preserve' }),
362+
);
363+
expect(refused?.code).toBe('STACK_CROSS_REFERENCE_INVALID');
364+
expect(refused?.issues).toContain(SEED_ON_NOWHERE);
365+
});
366+
367+
it('REFUSES a hand-built stack object on the same two rules', () => {
368+
const refused = refusalOf(() =>
369+
composeStacks([serviceStack(), handBuiltApp(NOWHERE, NOWHERE)], { manifest: 'preserve' }),
370+
);
371+
expect(refused?.code).toBe('STACK_CROSS_REFERENCE_INVALID');
372+
expect(refused?.issues).toContain(GRANT_ON_NOWHERE);
373+
expect(refused?.issues).toContain(SEED_ON_NOWHERE);
374+
});
375+
376+
it('leaves every OTHER rule un-applied to an unparsed input — only these two cross', () => {
377+
// A hook on an object nobody defines is refused by the PER-STACK pass only.
378+
// The artifact pass re-raises the two ARTIFACT-SCOPED rules and nothing
379+
// else, so an unparsed input carrying a dangling hook still composes.
380+
const unparsedHook = defineStack(
381+
anyStack({
382+
manifest: appManifest,
383+
objects: [account],
384+
hooks: [{ name: 'nowhere_hook', object: NOWHERE, events: ['afterInsert'], handler: 'noop' }],
385+
}),
386+
{ strict: false },
387+
);
388+
expect(refusalOf(() => composeStacks([serviceStack(), unparsedHook], { manifest: 'preserve' }))).toBeNull();
389+
});
390+
});
391+
392+
/**
393+
* The shape guard the two collectors carry (#18202 rework).
394+
*
395+
* Because the artifact pass reads `permissions` / `data` off inputs the strict
396+
* parse never saw, those keys can be a non-array, and an entry can be `null` or
397+
* a scalar. `composeStacks`'s step-3 concat pass already refuses to drop such a
398+
* key without a word (#5005); the two collectors must not turn the same input
399+
* into a bare `TypeError` with no `code` and no `status`, which is exactly what
400+
* this pass did before the guards existed. Every case below composes on
401+
* `origin/main`, so a throw here is a regression, not a stricter contract.
402+
*
403+
* ⚠️ `warnMalformedCollectionKey` deduplicates per key for the lifetime of the
404+
* module, so each key is asserted in exactly ONE test and the count assertion
405+
* (`toBe(1)`) is what proves the two passes do not both speak.
406+
*/
407+
describe('#18202 — a malformed collection on an unparsed input is skipped, never a bare TypeError', () => {
408+
/** Collect `console.warn` for one call, restoring the real one afterwards. */
409+
function warningsDuring(run: () => unknown): { warnings: string[]; thrown: Envelope | null } {
410+
const warnings: string[] = [];
411+
const real = console.warn;
412+
console.warn = (...args: unknown[]) => {
413+
warnings.push(args.map(String).join(' '));
414+
};
415+
try {
416+
return { warnings, thrown: refusalOf(run) };
417+
} finally {
418+
console.warn = real;
419+
}
420+
}
421+
422+
const malformed = (overrides: Record<string, unknown>) =>
423+
anyStack({ manifest: appManifest, objects: [account], ...overrides }) as unknown as ReturnType<typeof defineStack>;
424+
425+
const composeWith = (stack: ReturnType<typeof defineStack>) => () =>
426+
composeStacks([serviceStack(), stack], { manifest: 'preserve' });
427+
428+
it('a non-array `permissions` composes, and the key is warned about exactly once', () => {
429+
const { warnings, thrown } = warningsDuring(composeWith(malformed({ permissions: 'not-an-array' })));
430+
expect(thrown).toBeNull();
431+
expect(warnings.filter((w) => w.includes("top-level key 'permissions'"))).toHaveLength(1);
432+
});
433+
434+
it('a non-array `data` composes, and the key is warned about exactly once', () => {
435+
const { warnings, thrown } = warningsDuring(composeWith(malformed({ data: 42 })));
436+
expect(thrown).toBeNull();
437+
expect(warnings.filter((w) => w.includes("top-level key 'data'"))).toHaveLength(1);
438+
});
439+
440+
it('a null entry inside `permissions` is skipped, not dereferenced', () => {
441+
expect(refusalOf(composeWith(malformed({ permissions: [null] })))).toBeNull();
442+
});
443+
444+
it('a null entry inside `data` is skipped, not dereferenced', () => {
445+
expect(refusalOf(composeWith(malformed({ data: [null] })))).toBeNull();
446+
});
447+
448+
it('a scalar entry, and a non-string `object`, carry no reference for the rule to resolve', () => {
449+
expect(
450+
refusalOf(composeWith(malformed({ data: ['crm_case', { object: 7 }], permissions: ['sales_rep'] }))),
451+
).toBeNull();
452+
});
453+
454+
it('a malformed `objects` grant map is skipped while the rest of the set is still read', () => {
455+
const refused = refusalOf(
456+
composeWith(
457+
malformed({
458+
permissions: [
459+
{ name: 'broken', label: 'Broken', objects: null },
460+
{ name: 'sales_rep', label: 'Sales Rep', objects: { [NOWHERE]: { allowRead: true } } },
461+
],
462+
}),
463+
),
464+
);
465+
expect(refused?.code).toBe('STACK_CROSS_REFERENCE_INVALID');
466+
expect(refused?.issues).toContain(GRANT_ON_NOWHERE);
467+
});
468+
});
469+
470+
/**
471+
* The declared boundary of the artifact pass (#18202 rework).
472+
*
473+
* `composeStacks` returns `stacks[0]` untouched for a single input, so a
474+
* one-package composition never reaches the pass at all. That is why every
475+
* reader-facing statement of the guarantee — the option's docstring and the
476+
* multi-package docs bullet — says "in a composition of two or more packages".
477+
* This block is the fence on that qualifier: if the early return is ever
478+
* removed, the qualifier becomes wrong and these tests say so.
479+
*/
480+
describe('#18202 — a composition of ONE package never reaches the artifact pass', () => {
481+
const claiming = () => defineStack(appConfig(NOWHERE, NOWHERE), { artifactObjects: [NOWHERE] });
482+
483+
it('accepts a one-input composition whose claim names an object nothing defines', () => {
484+
expect(refusalOf(() => composeStacks([claiming()], { manifest: 'preserve' }))).toBeNull();
485+
});
486+
487+
it('accepts it with no options either — the early return precedes the option parse', () => {
488+
expect(refusalOf(() => composeStacks([claiming()]))).toBeNull();
489+
});
490+
491+
it('and REFUSES the identical claim as soon as a second package joins — the contrast', () => {
492+
const refused = refusalOf(() => composeStacks([serviceStack(), claiming()], { manifest: 'preserve' }));
493+
expect(refused?.code).toBe('STACK_CROSS_REFERENCE_INVALID');
494+
expect(refused?.issues).toContain(GRANT_ON_NOWHERE);
495+
});
496+
});

0 commit comments

Comments
 (0)