From cc4144a842514766eb54cceaa61289a7a613a704 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 17 Sep 2026 14:51:48 +0000 Subject: [PATCH 1/2] wip: import the artifact package-id owner in nav-contribution-groups Claude-Session: https://claude.ai/code/session_01DvvamiacK328idtBYJBxV3 Co-authored-by: Claude --- .../src/utils/nav-contribution-groups.test.ts | 147 +++++++++++++++++- .../cli/src/utils/nav-contribution-groups.ts | 67 +++++--- 2 files changed, 189 insertions(+), 25 deletions(-) diff --git a/packages/cli/src/utils/nav-contribution-groups.test.ts b/packages/cli/src/utils/nav-contribution-groups.test.ts index 043c0f27b56..73d2869939a 100644 --- a/packages/cli/src/utils/nav-contribution-groups.test.ts +++ b/packages/cli/src/utils/nav-contribution-groups.test.ts @@ -51,10 +51,11 @@ // The production import stays lazy and stays where it is: this line decides // only WHERE the first load is paid in THIS suite, and `os build`'s cold path // must not pull the data engine in to judge a stack with no contributions. -import '@objectstack/objectql/core'; +import { ObjectQL } from '@objectstack/objectql/core'; import { describe, it, expect } from 'vitest'; import { composeStacks, normalizeStackInput, ObjectStackDefinitionSchema } from '@objectstack/spec'; -import { artifactPackagesOf, collectNavGroupInputs, findNavGroupDiagnostics } from './nav-contribution-groups.js'; +import { artifactPackages } from './artifact-packages.js'; +import { collectNavGroupInputs, findNavGroupDiagnostics } from './nav-contribution-groups.js'; type AnyRec = Record; @@ -148,8 +149,13 @@ const parsedArtifact = (group: string): AnyRec => { * file keep passing while the rule the commands actually run drifted — which is * the same defect one layer down that the shared `checkNavContributionGroups` * exists to prevent. + * + * ⭐ [#18490] That is now the OWNER's rule and not a mirror of it: + * `nav-contribution-groups.ts` used to export its own `artifactPackagesOf`, + * which had already drifted from `artifactPackages` on an empty + * `manifest.id` + `manifest.name` (pinned at the foot of this file). */ -const packagesOf = (parsed: AnyRec) => artifactPackagesOf(parsed); +const packagesOf = (parsed: AnyRec) => artifactPackages(parsed); describe('#14553 — `os build` checks `navigationContributions[].group` across one composed artifact', () => { it('the fixture really is a two-package artifact — the floor under every reading below', async () => { @@ -175,7 +181,7 @@ describe('#14553 — `os build` checks `navigationContributions[].group` across it('needs only the parsed stack — the commands call it with one argument', async () => { // Both `os compile` and `os validate` reach this through // `findNavGroupDiagnostics(result.data)`, letting the package walk default - // to `artifactPackagesOf`. Pinned because the explicit-packages form is + // to `artifactPackages`. Pinned because the explicit-packages form is // what every other case here exercises, so a default that silently stopped // deriving would leave this file green while both commands went blind. const typod = artifact('sales_grp'); @@ -287,3 +293,136 @@ describe('#14553 — `os build` checks `navigationContributions[].group` across expect(found[0].group).toBe('nav_accounts'); }); }); + +/** + * #18490 — the package-id rule has ONE owner again, and importing it decided + * the one input the two copies disagreed on. + * + * `nav-contribution-groups.ts` used to carry its own `artifactPackagesOf`. It + * differed from `artifactPackages` in a single guard: a non-empty check on + * `manifest.name`. For a package whose `manifest.id` AND `manifest.name` are + * both `''` the owner answered `''` and the copy answered `packages[]`. + * Deleting the copy chose `''`, so what this block pins is not the string but + * the REASON it is right: `''` is what the runtime fold names that package, + * and `packages[]` is a spelling the runtime cannot produce at all. + * + * ⛔ Neither side's rule is re-spelled here. The build's answer comes out of + * the shipped `findNavGroupDiagnostics`, the runtime's out of a real + * `ObjectQL`, and the assertion is that the two STRINGS match — so the day + * either rule moves, this reds instead of agreeing with itself. + * + * `ObjectQL` is taken from `@objectstack/objectql/core`, the entry this file + * already loads at module top (see the note there): the pin adds assertions, + * not a module graph. + */ +describe('#18490 — the artifact package-id rule is the owner\'s, and both doors name one package alike', () => { + const EMPTY_PKG_NAMESPACE = 'ord'; + + /** The contributing package, with the divergent identity: both keys `''`. */ + const emptyIdOrdersStack = (group: string) => ({ + manifest: { + id: '', + name: '', + namespace: EMPTY_PKG_NAMESPACE, + version: '1.0.0', + type: 'module' as const, + navigationContributions: [{ + app: APP, + group, + items: [{ id: 'nav_orders', type: 'object' as const, objectName: 'ord_order', label: 'Orders' }], + }], + }, + objects: [{ + name: 'ord_order', + label: 'Order', + sharingModel: 'private' as const, + fields: { name: { name: 'name', type: 'text' as const, label: 'Order Number', required: true } }, + }], + }); + + /** Through the SAME parse chain `compile.ts` and `validate.ts` run. */ + const parsedEmptyIdArtifact = (group: string): AnyRec => { + const composed = composeStacks( + [emptyIdOrdersStack(group), coreStack()], + { manifest: 'preserve' }, + ) as unknown as Record; + const normalized = normalizeStackInput(composed, { onConversionNotice: () => {} }); + const result = ObjectStackDefinitionSchema.safeParse(normalized); + if (!result.success) { + throw new Error(`fixture does not parse: ${JSON.stringify(result.error.issues.slice(0, 3))}`); + } + return result.data as unknown as AnyRec; + }; + + it('an empty `manifest.id` AND `manifest.name` REACHES this check — the divergence is not hypothetical', () => { + // The floor under the two assertions below. `ManifestSchema` requires both + // keys as strings and constrains neither to be non-empty, so `''` parses — + // which is the only reason the two rules' disagreement was ever reachable + // from `os build` / `os validate` at all. If a future spec change refuses + // it, this reds FIRST and says the pins under it now measure nothing. + const parsed = parsedEmptyIdArtifact(GROUP); + const ids = packagesOf(parsed).map((pkg) => pkg.id).sort(); + expect(ids).toEqual(['', CORE_ID]); + }); + + it('the build names that package EXACTLY as the runtime fold does', async () => { + const parsed = parsedEmptyIdArtifact('sales_grp'); + const built = await findNavGroupDiagnostics(parsed); + expect(built).toHaveLength(1); + + // The same artifact installed into a real registry: `registerApp` derives + // the id it registers a contribution under, `getApp` runs the fold, and + // the fold records its own diagnostic for the relocation. + const engine = new ObjectQL(); + const core = coreStack(); + engine.registerApp({ ...core.manifest, apps: core.apps }); + engine.registerApp({ ...emptyIdOrdersStack('sales_grp').manifest }); + engine.registry.getApp(APP); + const folded = engine.registry.getAppNavDiagnostics(APP); + expect(folded).toHaveLength(1); + + // The whole point of the card, in two lines: one package, two doors, one + // name — the `packageId` field a consumer reads and the sentence an author + // reads. + expect(built[0].packageId).toBe(folded[0].packageId); + expect(built[0].message).toBe(folded[0].message); + }); + + it('⛔ and that shared name is NOT the deleted copy\'s positional spelling', async () => { + // Stated as its own assertion because the pin above would also pass if + // BOTH doors moved to `packages[0]`. The runtime has no positional + // fallback (`manifest.id || manifest.name`), so a build printing one is a + // build naming a package the runtime never will. + const parsed = parsedEmptyIdArtifact('sales_grp'); + const built = await findNavGroupDiagnostics(parsed); + expect(built[0].packageId).toBe(''); + expect(built[0].packageId).not.toBe('packages[0]'); + expect(built[0].message).not.toContain('packages[0]'); + }); + + it('two packages that both resolve to the empty id still produce TWO findings', async () => { + // The id is carried and printed, never used as a map key, a dedupe key or + // a sort key on this path. Pinned because "both collapse to one finding" + // is the failure mode an empty id would have if it ever became one, and it + // would present as a REPORT GOING QUIET rather than as an error. + const second = { + manifest: { + id: '', + name: '', + navigationContributions: [{ + app: APP, + group: 'sales_grp', + items: [{ id: 'nav_second', type: 'object' as const, objectName: 'ord_order', label: 'Second' }], + }], + }, + }; + const parsed = parsedEmptyIdArtifact('sales_grp'); + const widened: AnyRec = { + ...parsed, + packages: [...((parsed.packages ?? []) as unknown[]), second], + }; + const found = await findNavGroupDiagnostics(widened); + expect(found).toHaveLength(2); + expect(found.map((d) => d.packageId)).toEqual(['', '']); + }); +}); diff --git a/packages/cli/src/utils/nav-contribution-groups.ts b/packages/cli/src/utils/nav-contribution-groups.ts index d4b626b38c4..fe0c8f3bf05 100644 --- a/packages/cli/src/utils/nav-contribution-groups.ts +++ b/packages/cli/src/utils/nav-contribution-groups.ts @@ -39,6 +39,42 @@ * contribution: `os build`'s cold path should not pull the data engine in to * judge two empty arrays. * + * ## Why the package-id rule is imported too, and which answer that chose + * + * `artifactPackages` (`./artifact-packages.ts`) is the declared sole owner of + * "which package is this", and this module used to carry a second copy of it. + * The two had already drifted on one input — a package whose `manifest.id` AND + * `manifest.name` are both the empty string, which + * `ObjectStackDefinitionSchema` accepts (both keys are required strings on + * `ManifestSchema`; neither has a non-empty constraint, so `''` parses). The + * owner computed that package's id as `''`; the copy computed it as + * `packages[]`. + * + * ⭐ Importing the owner therefore CHOSE `''`, and that is the answer this path + * wants — measured against the runtime, which is the authority this module + * declares it mirrors two sections up. `ObjectQL.registerApp` derives the id it + * registers a contribution under as `manifest.id || manifest.name` and has NO + * positional fallback, so the fold registers that package under `''` and its + * relocation diagnostic reads `Package "" contributes …`. Under the deleted + * copy the build printed `Package "packages[0]" …` for the same artifact — + * two doors naming one package differently, which is the whole defect the + * shared `checkNavContributionGroups` exists to prevent, one field over. + * + * ⚠️ Two bounds on that reading, because they are where it could be wrong + * rather than merely narrow: + * + * - **The id is not a key here.** It is carried onto the diagnostic as + * `packageId` and printed; nothing on this path uses it as a map key, a + * dedupe key or a sort key, so two packages that both resolve to `''` still + * produce two findings rather than collapsing into one. Measured, not read. + * - **`artifactPackages` declares a precondition this path meets.** It reads + * the PARSED stack and does not re-check entry shape, so a `null` element + * would throw where the copy returned a positional id. Both commands call + * `findNavGroupDiagnostics(result.data)`, and every malformed element — + * `null`, a string, a non-object `manifest`, a missing one — is refused by + * `ArtifactPackageSchema` before that. ⛔ Do not hand this function a + * hand-built `packages[]` that has not been through the parse. + * * ## Why the findings ride the declared `warnings` key, and why BOTH commands * ## compute them * @@ -63,35 +99,24 @@ import type { NavContributionGroupDiagnostic } from '@objectstack/objectql'; +import { artifactPackages } from './artifact-packages.js'; + type AnyRec = Record; const asArray = (v: unknown): unknown[] => (Array.isArray(v) ? v : []); const asRec = (v: unknown): AnyRec | undefined => v && typeof v === 'object' && !Array.isArray(v) ? (v as AnyRec) : undefined; -/** One artifact package, in the `{ id, body }` shape the commands walk. */ -export interface CompiledPackage { - readonly id: string; - readonly body: AnyRec; -} - /** - * The artifact's package entries, derived from the PARSED stack. + * One artifact package, in the `{ id, body }` shape the commands walk — the + * shape `artifactPackages` returns, narrowed to the two facts read here. * - * Mirrors `compile.ts`' `artifactPackages` id rule — `manifest.id`, falling - * back to `name`, then to the positional spelling — because that is the string - * the runtime registers a contribution under, so a command names a package the - * same way the fold does. Derived here rather than passed in, so both commands - * reach the check through ONE call that needs only the parsed stack. + * ⛔ The id RULE is not re-spelled: this is a structural parameter type, and + * `./artifact-packages.ts` remains the only place that computes an id. */ -export function artifactPackagesOf(parsed: AnyRec): CompiledPackage[] { - return asArray(parsed.packages).map((entry, index) => { - const body = asRec((entry as { manifest?: unknown })?.manifest) ?? {}; - const id = typeof body.id === 'string' && body.id !== '' - ? body.id - : (typeof body.name === 'string' && body.name !== '' ? body.name : `packages[${index}]`); - return { id, body }; - }); +export interface CompiledPackage { + readonly id: string; + readonly body: AnyRec; } /** @@ -170,7 +195,7 @@ export function collectNavGroupInputs( */ export async function findNavGroupDiagnostics( parsed: AnyRec, - packages: readonly CompiledPackage[] = artifactPackagesOf(parsed), + packages: readonly CompiledPackage[] = artifactPackages(parsed), ): Promise { const { apps, contributions } = collectNavGroupInputs(parsed, packages); if (contributions.length === 0 || apps.length === 0) return []; From 6a886b8a4c6271639400cf4d4400e0ba97ab9d1b Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 17 Sep 2026 15:32:35 +0000 Subject: [PATCH 2/2] 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 --- ...ibution-groups-imports-package-id-owner.md | 16 ++ ...nav-contribution-groups.package-id.test.ts | 195 ++++++++++++++++++ .../src/utils/nav-contribution-groups.test.ts | 135 +----------- 3 files changed, 212 insertions(+), 134 deletions(-) create mode 100644 .changeset/18490-nav-contribution-groups-imports-package-id-owner.md create mode 100644 packages/cli/src/utils/nav-contribution-groups.package-id.test.ts diff --git a/.changeset/18490-nav-contribution-groups-imports-package-id-owner.md b/.changeset/18490-nav-contribution-groups-imports-package-id-owner.md new file mode 100644 index 00000000000..8c0d17c521b --- /dev/null +++ b/.changeset/18490-nav-contribution-groups-imports-package-id-owner.md @@ -0,0 +1,16 @@ +--- +"@objectstack/cli": patch +--- + +`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). + +Clause-②: no + +`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. + +- **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[]` ``. +- **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. +- **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[]`, 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. +- **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. + +`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`. diff --git a/packages/cli/src/utils/nav-contribution-groups.package-id.test.ts b/packages/cli/src/utils/nav-contribution-groups.package-id.test.ts new file mode 100644 index 00000000000..bc8e7b2ef58 --- /dev/null +++ b/packages/cli/src/utils/nav-contribution-groups.package-id.test.ts @@ -0,0 +1,195 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #18490 — the artifact package-id rule has ONE owner again, and importing it + * DECIDED the one input the two copies disagreed on. + * + * ## What this file measures, and against what + * + * `packages/cli/src/utils/artifact-packages.ts` declares itself the sole owner + * of "which package is this". `nav-contribution-groups.ts` carried a second + * implementation, `artifactPackagesOf`, differing from the owner in a single + * guard — a non-empty check on `manifest.name`. For a package whose + * `manifest.id` AND `manifest.name` are both `''` the owner answered `''` and + * the copy answered `packages[]`. + * + * Deleting the copy chose `''`. What this file pins is not that string but the + * REASON it is the right one: `''` is what the RUNTIME fold names that package, + * and `packages[]` is a spelling the runtime cannot produce at all + * (`ObjectQL.registerApp` derives `manifest.id || manifest.name`, with no + * positional fallback). `nav-contribution-groups.ts`'s own header says the id + * it carries is "the string the runtime registers a contribution under, so a + * command names a package the same way the fold does" — this is that sentence, + * measured. + * + * ⛔ NEITHER side's rule is re-spelled here. The build's answer comes out of + * the shipped `findNavGroupDiagnostics`, the runtime's out of a real + * `ObjectQL`, and the assertion is that the two STRINGS match — so the day + * either rule moves, this reds instead of agreeing with itself. + * + * ## Why this is a file of its own, and why it is INTEGRATION tier + * + * The cross-door half constructs `new ObjectQL(`, which is a KERNEL signal in + * `vitest-tiers.ts`'s predicate — so a file carrying it is integration tier by + * derivation, not by naming. Folding these cases into the unit-tier + * `nav-contribution-groups.test.ts` would have moved that whole file, and its + * nine existing #14553 pins with it, out of the tier they were written for. + * Splitting keeps the tier change confined to the cases that actually boot a + * registry. ⛔ Do not merge this file back into that one. + */ + +import { describe, it, expect } from 'vitest'; +import { composeStacks, normalizeStackInput, ObjectStackDefinitionSchema } from '@objectstack/spec'; +import { ObjectQL } from '@objectstack/objectql/core'; +import { findNavGroupDiagnostics } from './nav-contribution-groups.js'; +import { artifactPackages } from './artifact-packages.js'; + +type AnyRec = Record; + +const APP = 'multi_crm'; +const GROUP = 'sales_group'; +const TYPO = 'sales_grp'; +const CORE_ID = 'com.example.multi.core'; + +/** The App package — owns the app and the group container the module aims at. */ +const coreStack = () => ({ + manifest: { + id: CORE_ID, + name: 'Multi-Package Core', + namespace: 'crm', + version: '1.0.0', + type: 'app' as const, + }, + objects: [{ + name: 'crm_account', + label: 'Account', + sharingModel: 'private' as const, + fields: { name: { name: 'name', type: 'text' as const, label: 'Account Name', required: true } }, + }], + apps: [{ + name: APP, + label: 'Multi-Package CRM', + navigation: [{ + id: GROUP, + type: 'group' as const, + label: 'Sales', + children: [{ id: 'nav_accounts', type: 'object' as const, objectName: 'crm_account', label: 'Accounts' }], + }], + }], +}); + +/** + * The contributing package, carrying the divergent identity: both id keys `''`. + * + * ⚠️ Its own namespace, not the app package's. The composed-artifact leg does + * not care, but the runtime leg INSTALLS both packages into one registry and + * ADR-0048 refuses a second package under a namespace another already owns — + * a conflict that would make this file red for a reason that has nothing to do + * with what it measures. + */ +const emptyIdOrdersStack = (group: string) => ({ + manifest: { + id: '', + name: '', + namespace: 'ord', + version: '1.0.0', + type: 'module' as const, + navigationContributions: [{ + app: APP, + group, + items: [{ id: 'nav_orders', type: 'object' as const, objectName: 'ord_order', label: 'Orders' }], + }], + }, + objects: [{ + name: 'ord_order', + label: 'Order', + sharingModel: 'private' as const, + fields: { name: { name: 'name', type: 'text' as const, label: 'Order Number', required: true } }, + }], +}); + +/** The artifact as the commands actually hand it down — through their own parse. */ +const parsedEmptyIdArtifact = (group: string): AnyRec => { + const composed = composeStacks( + [emptyIdOrdersStack(group), coreStack()], + { manifest: 'preserve' }, + ) as unknown as Record; + const normalized = normalizeStackInput(composed, { onConversionNotice: () => {} }); + const result = ObjectStackDefinitionSchema.safeParse(normalized); + if (!result.success) { + throw new Error(`fixture does not parse: ${JSON.stringify(result.error.issues.slice(0, 3))}`); + } + return result.data as unknown as AnyRec; +}; + +describe('#18490 — one package, two doors, one name', () => { + it('an empty `manifest.id` AND `manifest.name` REACHES this check — the divergence is not hypothetical', () => { + // The floor under everything below. `ManifestSchema` requires both keys as + // strings and constrains NEITHER to be non-empty, so `''` parses — which is + // the only reason the two rules could ever disagree on a stack `os build` + // or `os validate` would actually accept. If a spec change starts refusing + // it, this reds FIRST and says the pins under it now measure nothing. + const parsed = parsedEmptyIdArtifact(GROUP); + expect(artifactPackages(parsed).map((pkg) => pkg.id).sort()).toEqual(['', CORE_ID]); + }); + + it('the build names that package EXACTLY as the runtime fold does', async () => { + // The build door: the shipped derivation, reached the way both commands + // reach it — `findNavGroupDiagnostics(result.data)`, one argument. + const built = await findNavGroupDiagnostics(parsedEmptyIdArtifact(TYPO)); + expect(built).toHaveLength(1); + + // The runtime door: the same two packages installed into a real registry. + // `registerApp` derives the id it registers the contribution under, the + // read-time fold relocates the mis-aimed items, and recording that + // relocation is what produces the diagnostic. + const engine = new ObjectQL(); + const core = coreStack(); + engine.registerApp({ ...core.manifest, apps: core.apps }); + engine.registerApp({ ...emptyIdOrdersStack(TYPO).manifest }); + engine.registry.getApp(APP); + const folded = engine.registry.getAppNavDiagnostics(APP); + expect(folded).toHaveLength(1); + + // The card, in two lines: the `packageId` field a consumer reads, and the + // sentence an author reads. + expect(built[0].packageId).toBe(folded[0].packageId); + expect(built[0].message).toBe(folded[0].message); + }); + + it('⛔ and that shared name is NOT the deleted copy\'s positional spelling', async () => { + // Its own assertion, because the pin above would also pass if BOTH doors + // moved to `packages[0]`. The runtime has no positional fallback, so a + // build printing one is a build naming a package the runtime never will. + const built = await findNavGroupDiagnostics(parsedEmptyIdArtifact(TYPO)); + expect(built[0].packageId).toBe(''); + expect(built[0].packageId).not.toBe('packages[0]'); + expect(built[0].message).not.toContain('packages[0]'); + }); + + it('two packages that both resolve to the empty id still produce TWO findings', async () => { + // The id is CARRIED and PRINTED on this path — never a map key, a dedupe + // key or a sort key. Pinned because "both collapse into one finding" is the + // failure an empty id would cause if it ever became one, and it would + // present as the report going QUIET rather than as an error. + const second = { + manifest: { + id: '', + name: '', + navigationContributions: [{ + app: APP, + group: TYPO, + items: [{ id: 'nav_second', type: 'object' as const, objectName: 'ord_order', label: 'Second' }], + }], + }, + }; + const parsed = parsedEmptyIdArtifact(TYPO); + const widened: AnyRec = { + ...parsed, + packages: [...((parsed.packages ?? []) as unknown[]), second], + }; + const found = await findNavGroupDiagnostics(widened); + expect(found).toHaveLength(2); + expect(found.map((d) => d.packageId)).toEqual(['', '']); + }); +}); diff --git a/packages/cli/src/utils/nav-contribution-groups.test.ts b/packages/cli/src/utils/nav-contribution-groups.test.ts index 73d2869939a..d82a769f548 100644 --- a/packages/cli/src/utils/nav-contribution-groups.test.ts +++ b/packages/cli/src/utils/nav-contribution-groups.test.ts @@ -51,7 +51,7 @@ // The production import stays lazy and stays where it is: this line decides // only WHERE the first load is paid in THIS suite, and `os build`'s cold path // must not pull the data engine in to judge a stack with no contributions. -import { ObjectQL } from '@objectstack/objectql/core'; +import '@objectstack/objectql/core'; import { describe, it, expect } from 'vitest'; import { composeStacks, normalizeStackInput, ObjectStackDefinitionSchema } from '@objectstack/spec'; import { artifactPackages } from './artifact-packages.js'; @@ -293,136 +293,3 @@ describe('#14553 — `os build` checks `navigationContributions[].group` across expect(found[0].group).toBe('nav_accounts'); }); }); - -/** - * #18490 — the package-id rule has ONE owner again, and importing it decided - * the one input the two copies disagreed on. - * - * `nav-contribution-groups.ts` used to carry its own `artifactPackagesOf`. It - * differed from `artifactPackages` in a single guard: a non-empty check on - * `manifest.name`. For a package whose `manifest.id` AND `manifest.name` are - * both `''` the owner answered `''` and the copy answered `packages[]`. - * Deleting the copy chose `''`, so what this block pins is not the string but - * the REASON it is right: `''` is what the runtime fold names that package, - * and `packages[]` is a spelling the runtime cannot produce at all. - * - * ⛔ Neither side's rule is re-spelled here. The build's answer comes out of - * the shipped `findNavGroupDiagnostics`, the runtime's out of a real - * `ObjectQL`, and the assertion is that the two STRINGS match — so the day - * either rule moves, this reds instead of agreeing with itself. - * - * `ObjectQL` is taken from `@objectstack/objectql/core`, the entry this file - * already loads at module top (see the note there): the pin adds assertions, - * not a module graph. - */ -describe('#18490 — the artifact package-id rule is the owner\'s, and both doors name one package alike', () => { - const EMPTY_PKG_NAMESPACE = 'ord'; - - /** The contributing package, with the divergent identity: both keys `''`. */ - const emptyIdOrdersStack = (group: string) => ({ - manifest: { - id: '', - name: '', - namespace: EMPTY_PKG_NAMESPACE, - version: '1.0.0', - type: 'module' as const, - navigationContributions: [{ - app: APP, - group, - items: [{ id: 'nav_orders', type: 'object' as const, objectName: 'ord_order', label: 'Orders' }], - }], - }, - objects: [{ - name: 'ord_order', - label: 'Order', - sharingModel: 'private' as const, - fields: { name: { name: 'name', type: 'text' as const, label: 'Order Number', required: true } }, - }], - }); - - /** Through the SAME parse chain `compile.ts` and `validate.ts` run. */ - const parsedEmptyIdArtifact = (group: string): AnyRec => { - const composed = composeStacks( - [emptyIdOrdersStack(group), coreStack()], - { manifest: 'preserve' }, - ) as unknown as Record; - const normalized = normalizeStackInput(composed, { onConversionNotice: () => {} }); - const result = ObjectStackDefinitionSchema.safeParse(normalized); - if (!result.success) { - throw new Error(`fixture does not parse: ${JSON.stringify(result.error.issues.slice(0, 3))}`); - } - return result.data as unknown as AnyRec; - }; - - it('an empty `manifest.id` AND `manifest.name` REACHES this check — the divergence is not hypothetical', () => { - // The floor under the two assertions below. `ManifestSchema` requires both - // keys as strings and constrains neither to be non-empty, so `''` parses — - // which is the only reason the two rules' disagreement was ever reachable - // from `os build` / `os validate` at all. If a future spec change refuses - // it, this reds FIRST and says the pins under it now measure nothing. - const parsed = parsedEmptyIdArtifact(GROUP); - const ids = packagesOf(parsed).map((pkg) => pkg.id).sort(); - expect(ids).toEqual(['', CORE_ID]); - }); - - it('the build names that package EXACTLY as the runtime fold does', async () => { - const parsed = parsedEmptyIdArtifact('sales_grp'); - const built = await findNavGroupDiagnostics(parsed); - expect(built).toHaveLength(1); - - // The same artifact installed into a real registry: `registerApp` derives - // the id it registers a contribution under, `getApp` runs the fold, and - // the fold records its own diagnostic for the relocation. - const engine = new ObjectQL(); - const core = coreStack(); - engine.registerApp({ ...core.manifest, apps: core.apps }); - engine.registerApp({ ...emptyIdOrdersStack('sales_grp').manifest }); - engine.registry.getApp(APP); - const folded = engine.registry.getAppNavDiagnostics(APP); - expect(folded).toHaveLength(1); - - // The whole point of the card, in two lines: one package, two doors, one - // name — the `packageId` field a consumer reads and the sentence an author - // reads. - expect(built[0].packageId).toBe(folded[0].packageId); - expect(built[0].message).toBe(folded[0].message); - }); - - it('⛔ and that shared name is NOT the deleted copy\'s positional spelling', async () => { - // Stated as its own assertion because the pin above would also pass if - // BOTH doors moved to `packages[0]`. The runtime has no positional - // fallback (`manifest.id || manifest.name`), so a build printing one is a - // build naming a package the runtime never will. - const parsed = parsedEmptyIdArtifact('sales_grp'); - const built = await findNavGroupDiagnostics(parsed); - expect(built[0].packageId).toBe(''); - expect(built[0].packageId).not.toBe('packages[0]'); - expect(built[0].message).not.toContain('packages[0]'); - }); - - it('two packages that both resolve to the empty id still produce TWO findings', async () => { - // The id is carried and printed, never used as a map key, a dedupe key or - // a sort key on this path. Pinned because "both collapse to one finding" - // is the failure mode an empty id would have if it ever became one, and it - // would present as a REPORT GOING QUIET rather than as an error. - const second = { - manifest: { - id: '', - name: '', - navigationContributions: [{ - app: APP, - group: 'sales_grp', - items: [{ id: 'nav_second', type: 'object' as const, objectName: 'ord_order', label: 'Second' }], - }], - }, - }; - const parsed = parsedEmptyIdArtifact('sales_grp'); - const widened: AnyRec = { - ...parsed, - packages: [...((parsed.packages ?? []) as unknown[]), second], - }; - const found = await findNavGroupDiagnostics(widened); - expect(found).toHaveLength(2); - expect(found.map((d) => d.packageId)).toEqual(['', '']); - }); -});