From 63764a2feab17d9fd81bfbed8713ca17799d9cd5 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 3 Sep 2026 09:33:27 +0000 Subject: [PATCH 1/8] feat(objectql,cli): make a relocated navigation contribution a real diagnostic MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `SchemaRegistry.applyNavContributions` relocates a contribution whose `group` names no group in the target app to the app's top level. That stays — the read-time fold is order-independent by design and contributions into optional groups must keep working — but the only trace was one `log()` call gated at `info`/`debug`, so at `OS_REGISTRY_LOG=warn` an app's information architecture changed in complete silence. The trace is now an ADR-0038 BuildIssue-family record (ADR-0112 D6c: a diagnostics code, lowercase and out of the error ledger) naming the contributing package, the app, the missing group id and the relocated items. It is carried on the app (`getAppNavDiagnostics`) and announced through a new level-aware `warn()`, once per registry per distinct mis-aim. `os build` answers the same question at compile time over a composed artifact, through the same predicate, and reports it loudly without refusing. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01UHvF5hyiZjnCyExFnfQB8m --- .../src/packages/core/index.ts | 14 +- .../src/packages/orders/index.ts | 29 +++ packages/cli/src/commands/compile.ts | 54 ++++ .../src/utils/nav-contribution-groups.test.ts | 230 +++++++++++++++++ .../cli/src/utils/nav-contribution-groups.ts | 139 +++++++++++ packages/objectql/src/core.ts | 20 ++ packages/objectql/src/index.ts | 19 ++ .../src/nav-contribution-diagnostics.ts | 213 ++++++++++++++++ ...y-nav-contribution-group-semantics.test.ts | 231 +++++++++++++++--- packages/objectql/src/registry.ts | 119 ++++++++- 10 files changed, 1022 insertions(+), 46 deletions(-) create mode 100644 packages/cli/src/utils/nav-contribution-groups.test.ts create mode 100644 packages/cli/src/utils/nav-contribution-groups.ts create mode 100644 packages/objectql/src/nav-contribution-diagnostics.ts diff --git a/examples/app-multi-package/src/packages/core/index.ts b/examples/app-multi-package/src/packages/core/index.ts index 5508dac9b0..0f2815bc18 100644 --- a/examples/app-multi-package/src/packages/core/index.ts +++ b/examples/app-multi-package/src/packages/core/index.ts @@ -43,8 +43,20 @@ export default defineStack({ name: 'multi_crm', label: 'Multi-Package CRM', description: 'Accounts, plus whatever modules this artifact delivers alongside', + // The group is a CONTAINER this package owns and modules aim at + // (ADR-0029 D7). It is the App package's half of the split: a module + // cannot declare a group inside an app it does not own, so the app has + // to publish the container its modules contribute into — which is what + // makes `navigationContributions[].group` resolvable at all. navigation: [ - { id: 'nav_accounts', type: 'object', objectName: 'crm_account', label: 'Accounts', icon: 'building' }, + { + id: 'sales_group', + type: 'group', + label: 'Sales', + children: [ + { id: 'nav_accounts', type: 'object', objectName: 'crm_account', label: 'Accounts', icon: 'building' }, + ], + }, ], }, ], diff --git a/examples/app-multi-package/src/packages/orders/index.ts b/examples/app-multi-package/src/packages/orders/index.ts index 096c8a18cd..46ea1919c8 100644 --- a/examples/app-multi-package/src/packages/orders/index.ts +++ b/examples/app-multi-package/src/packages/orders/index.ts @@ -32,6 +32,22 @@ import { defineStack } from '@objectstack/spec'; * legal and is the whole point of the split: cross-package lookups are accepted * (ADR-0130 §1.5), while a package's own app navigation pointing at a foreign * object is not — which is why the navigation lives with the App package. + * + * ## Why it also carries a `navigationContributions` entry (#14553) + * + * The other half of that same rule. R3 refuses an app's OWN `navigation` entry + * naming another package's object, so a module split converts every such entry + * into a contribution owned by the module — which is exactly what this one is: + * `crm_order` is reachable from the App's menu without the App package + * knowing the object exists. + * + * ⚠️ `group` names `sales_group`, a container the CORE package declares. A + * module cannot see that id at authoring time, and a typo in it does not fail: + * the runtime RELOCATES the items to the app's top level and says so + * (`nav_contribution_group_missing`, at `warn`), and `os build` reports the + * same finding at compile time. This fixture is where that is measured — keep + * the id spelled correctly here, so a build of this example stays clean and the + * pin that typos it has something to differ from. */ export default defineStack({ manifest: { @@ -46,6 +62,19 @@ export default defineStack({ // it as the topological edge that registers core BEFORE orders (ADR-0130 // D5, ADR-0116's one sorter) — the array order below is not what decides. dependencies: { 'com.example.multi.core': '^1.0.0' }, + + // ADR-0029 D7 — `navigationContributions` is a MANIFEST key, not a stack + // collection: it describes what this PACKAGE injects into someone else's + // app, so it travels with the package identity. + navigationContributions: [ + { + app: 'multi_crm', + group: 'sales_group', + items: [ + { id: 'nav_orders', type: 'object', objectName: 'crm_order', label: 'Orders', icon: 'shopping-cart' }, + ], + }, + ], }, objects: [ diff --git a/packages/cli/src/commands/compile.ts b/packages/cli/src/commands/compile.ts index c1f3d0453f..86304554c8 100644 --- a/packages/cli/src/commands/compile.ts +++ b/packages/cli/src/commands/compile.ts @@ -42,6 +42,10 @@ import { errorCodeFields, } from '../utils/format.js'; import { checkProtocolVersionGap } from '../utils/protocol-version-gap.js'; +// [#14553] The compile-time half of the navigation-contribution group ruling. +// Reports; never refuses — the runtime still relocates, deliberately. +import { findNavGroupDiagnostics } from '../utils/nav-contribution-groups.js'; +import type { NavContributionGroupDiagnostic } from '@objectstack/objectql'; /** * The artifact's package entries, as `{ index, id, body }` (ADR-0130 D4). @@ -177,6 +181,12 @@ export default class Compile extends Command { let capProviderWarnings: Array<{ token: string; message: string }> = []; let unknownKeyWarnings: string[] = []; let docWarnings: DocIssue[] = []; + // [#14553] Build-only, so a SEPARATE payload key rather than a member of + // `warningsSoFar()` — the `bodyExtractionWarnings` precedent one field + // over, and for its stated reason: `os validate` never computes these, and + // folding a shape only ONE command can ship into the cross-command + // `warnings` key teaches consumers a shape the other command never emits. + let navGroupDiagnostics: NavContributionGroupDiagnostic[] = []; const warningsSoFar = () => [ ...ruleAdvisories, ...docWarnings, @@ -446,6 +456,40 @@ export default class Compile extends Command { } } + // 3b-bis. [#14553] Navigation contributions whose `group` names no group + // in the target app. RUNS ON EVERY BUILD, artifact or not — the block + // above is skipped for a single-package stack, but a stack that + // declares an app AND contributes into it has the identical defect + // and `collectNavGroupInputs` reads it from the top-level manifest. + // + // ⛔ REPORTS, NEVER REFUSES. The maintainer ruled option B: the + // runtime keeps relocating the items to the app's top level (the fold + // stays order-independent, contributions into optional groups keep + // working) and the failure becomes VISIBLE instead. Making this exit + // non-zero would be option A wearing a warning's clothes, and would + // narrow what `os build` accepts — which the ruling explicitly does + // not do. + // + // A contribution whose target app is NOT in this compilation unit + // yields nothing: contributing into an app another artifact ships is + // the supported cross-artifact case, and is precisely why the merge + // is a read-time fold. Only the composed case can be judged here. + navGroupDiagnostics = await findNavGroupDiagnostics( + result.data as Record, + packageEntries, + ); + if (navGroupDiagnostics.length > 0 && !flags.json) { + console.log(''); + printWarning( + `Navigation contributions aimed at a group the target app does not declare ` + + `(${navGroupDiagnostics.length}) — the items still install, RELOCATED to the app's top level`, + ); + printBulletList( + navGroupDiagnostics.map((d) => `[${d.code}] ${d.message} Fix: ${d.fix}`), + { noun: 'navigation-contribution diagnostic' }, + ); + } + // 3c. [#3366] Installable-provider preflight. Every capability the app // DECLARES in `requires: [...]` must have a provider resolvable in the // active edition. A `requires` entry whose provider has NO installable @@ -795,6 +839,16 @@ export default class Compile extends Command { // callable lowered cleanly, so a CI consumer can read the key // unconditionally. bodyExtractionWarnings: lowering.bodyExtractionWarnings, + // [#14553] Navigation contributions relocated past a missing group. + // A SEPARATE key for the same reason as the one above: `os validate` + // computes nothing of the kind, so folding these records into the + // cross-command `warnings` list would teach a consumer a shape only + // this command can ever ship. Empty array when every contribution + // resolved, so CI can read the key unconditionally — the records are + // the ADR-0038 BuildIssue-family entries the runtime fold raises + // (ADR-0112 D6c: a diagnostics code, lowercase and out of the error + // ledger), so one consumer reads one shape from either door. + navigationGroupDiagnostics: navGroupDiagnostics, // Same key `os validate --json` uses, so a CI consumer reads one shape // from either command rather than learning two. conversions: conversionNotices, diff --git a/packages/cli/src/utils/nav-contribution-groups.test.ts b/packages/cli/src/utils/nav-contribution-groups.test.ts new file mode 100644 index 0000000000..fc04b7dd1f --- /dev/null +++ b/packages/cli/src/utils/nav-contribution-groups.test.ts @@ -0,0 +1,230 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #14553 — `os build`'s compile-time half: a `navigationContributions[].group` + * that names no group in an app the SAME artifact ships. + * + * ## What this file measures, and against what + * + * The maintainer ruled (2026-09-02, verbatim 「同意」) that the runtime keeps + * relocating such a contribution to the app's top level and says so at `warn`, + * and that `os build` reports the same finding at compile time "where an AI + * author sees it first". The runtime half is pinned in + * `packages/objectql/src/registry-nav-contribution-group-semantics.test.ts`; + * this file is the build half. + * + * The inputs run through the REAL `composeStacks(…, { manifest: 'preserve' })` + * rather than a hand-written artifact literal, because the whole question is + * what a COMPOSED artifact looks like — a literal would pin this check against + * a shape nothing produces, and would go on passing the day composition + * changes. They mirror `examples/app-multi-package`, which is the fixture the + * ruling names, including its two deliberate properties (the module is listed + * first; it declares `dependencies` on the app package). + * + * ⚠️ MIRRORED, not imported. `packages/metadata`'s artifact-attribution suite + * takes the same approach for the same example and states the reason: a test + * reaching outside its own package for a fixture is invisible to + * `turbo ls --affected` and to the `test` task's input hashing unless it is + * spelled the way `check:cross-package-test-inputs` recognises, and the shape + * under test here is three keys deep. The example is edited in the same PR to + * carry the correct spelling, so the fixture below and the example agree by + * construction; what this file pins is the DERIVATION, and `sales_group` / + * `multi_crm` are named identically in both so a reader can diff them by eye. + * + * ## Why the typo is applied to the composed artifact and not authored twice + * + * Both legs compose from ONE pair of stacks and differ in a single character + * position — the `group` id — so nothing else can drift between the clean and + * the typo'd reading. A second hand-authored fixture would be a second thing to + * keep in sync, and a divergence in it would read as a finding. + */ + +import { describe, it, expect, beforeAll } from 'vitest'; +import { composeStacks } from '@objectstack/spec'; +import { collectNavGroupInputs, findNavGroupDiagnostics } from './nav-contribution-groups.js'; + +type AnyRec = Record; + +const APP = 'multi_crm'; +const GROUP = 'sales_group'; +const CORE_ID = 'com.example.multi.core'; +const ORDERS_ID = 'com.example.multi.orders'; + +/** The App package — owns the app and the group container modules aim 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 Module package — contributes into the App package's group. */ +const ordersStack = (group: string) => ({ + manifest: { + id: ORDERS_ID, + name: 'Multi-Package Orders', + namespace: 'crm', + version: '1.0.0', + type: 'module' as const, + dependencies: { [CORE_ID]: '^1.0.0' }, + navigationContributions: [{ + app: APP, + group, + items: [{ id: 'nav_orders', type: 'object', objectName: 'crm_order', label: 'Orders' }], + }], + }, + objects: [{ + name: 'crm_order', + label: 'Order', + sharingModel: 'private' as const, + fields: { name: { name: 'name', type: 'text' as const, label: 'Order Number', required: true } }, + }], +}); + +/** Compose exactly as `examples/app-multi-package/objectstack.config.ts` does. */ +const artifact = (group: string): AnyRec => + composeStacks([ordersStack(group), coreStack()], { manifest: 'preserve' }) as unknown as AnyRec; + +/** The artifact's packages, in the `{ index, id, body }` shape `compile.ts` walks. */ +const packagesOf = (parsed: AnyRec) => + ((parsed.packages ?? []) as Array<{ manifest?: AnyRec }>).map((entry, index) => { + const body = (entry.manifest ?? {}) as AnyRec; + const id = typeof body.id === 'string' && body.id !== '' + ? body.id + : (typeof body.name === 'string' ? body.name : `packages[${index}]`); + return { id, body }; + }); + +describe('#14553 — `os build` checks `navigationContributions[].group` across one composed artifact', () => { + // `findNavGroupDiagnostics` loads `@objectstack/objectql/core` LAZILY, and + // that first load costs several seconds — measured here, over vitest's 5s + // default, which is what surfaced it. That cost is the reason the import is + // lazy in production too: `os build`'s cold path must not pull the data + // engine in to judge a stack that declares no contributions at all. Warmed + // once, with its own budget, so the per-test durations below report what the + // derivation costs rather than what the module loader does. + beforeAll(async () => { + await import('@objectstack/objectql/core'); + }, 60_000); + + it('the fixture really is a two-package artifact — the floor under every reading below', async () => { + // Without this, a composition change that stopped emitting `packages[]` + // would make every assertion in this file vacuously true: the derivation + // would see no contributions and report nothing, which is also what "clean" + // looks like. Asserted first, and on the CLEAN leg, so the file cannot go + // green by measuring an empty artifact. + const parsed = artifact(GROUP); + const packages = packagesOf(parsed); + expect(packages.map((p) => p.id).sort()).toEqual([CORE_ID, ORDERS_ID]); + + const { apps, contributions } = collectNavGroupInputs(parsed, packages); + expect(apps.map((a) => a.name)).toContain(APP); + expect(contributions).toHaveLength(1); + expect(contributions[0]).toMatchObject({ app: APP, group: GROUP, packageId: ORDERS_ID }); + }); + + it('the correctly spelled group id reports NOTHING', async () => { + expect(await findNavGroupDiagnostics(artifact(GROUP), packagesOf(artifact(GROUP)))).toEqual([]); + }); + + it('ONE typo\'d group id prints one diagnostic naming the package, the app, the group and the items', async () => { + const parsed = artifact('sales_grp'); + const found = await findNavGroupDiagnostics(parsed, packagesOf(parsed)); + + expect(found).toHaveLength(1); + expect(found[0]).toMatchObject({ + code: 'nav_contribution_group_missing', + severity: 'warning', + app: APP, + packageId: ORDERS_ID, + group: 'sales_grp', + relocated: ['nav_orders'], + }); + // The four facts the ruling required, in the printed text — this is the + // string an author actually reads, and `severity: 'warning'` is what says + // the build still succeeds. + expect(found[0].message).toContain(ORDERS_ID); + expect(found[0].message).toContain(APP); + expect(found[0].message).toContain('sales_grp'); + expect(found[0].message).toContain('nav_orders'); + expect(found[0].fix).toContain('sales_grp'); + }); + + it('reports the mis-aim ONCE, not once per surface the composed artifact carries it on', async () => { + // `manifest: 'preserve'` is ADDITIVE: it flattens every collection to the + // top level AND still picks a singular `manifest` by the default `'last'` + // rule. So the contribution is reachable twice in a composed artifact — + // once under `packages[]`, once under the picked top-level manifest — and a + // walk that read both would report the same author error twice, the second + // time with no package id to act on. + const parsed = artifact('sales_grp'); + expect(await findNavGroupDiagnostics(parsed, packagesOf(parsed))).toHaveLength(1); + }); + + it('a contribution into an app NO package here ships is not a finding — the cross-artifact case stays supported', async () => { + // The bound that keeps this a report rather than option A. A package may + // legally contribute into an app shipped by a different artifact installed + // separately — that is precisely why the merge is a read-time fold — so the + // only mis-aims a build can judge are the ones it can see both halves of. + const parsed = artifact(GROUP); + (parsed as AnyRec).packages = ((parsed.packages ?? []) as unknown[]).slice(); + const orders = ordersStack(GROUP); + const found = await findNavGroupDiagnostics( + { packages: [] }, + [{ id: ORDERS_ID, body: { ...orders.manifest } as AnyRec }], + ); + expect(found).toEqual([]); + }); + + it('a SINGLE-package stack is judged too — the same defect, without an artifact', async () => { + // `packages[]` is absent from an ordinary `defineStack` project, so the + // artifact walk skips it entirely. A stack that declares an app and + // contributes into it has the identical mis-aim, and the top-level manifest + // is where its contributions live. + const single: AnyRec = { + manifest: { + id: 'com.example.single', + name: 'Single', + navigationContributions: [{ + app: APP, + group: 'sales_grp', + items: [{ id: 'nav_orders', type: 'object', objectName: 'crm_order', label: 'Orders' }], + }], + }, + apps: coreStack().apps, + }; + const found = await findNavGroupDiagnostics(single, []); + expect(found).toHaveLength(1); + expect(found[0]).toMatchObject({ app: APP, group: 'sales_grp', packageId: 'com.example.single' }); + }); + + it('naming an id that exists but is not a `type: "group"` node reaches the same diagnostic', async () => { + // `findNavGroup` requires BOTH `id` and `type === 'group'`, so pointing at + // the app's `nav_accounts` object entry is the same authoring error as + // pointing at nothing — and the build must not let the near-miss through + // just because the id resolves to something. + const parsed = artifact('nav_accounts'); + const found = await findNavGroupDiagnostics(parsed, packagesOf(parsed)); + expect(found).toHaveLength(1); + expect(found[0].group).toBe('nav_accounts'); + }); +}); diff --git a/packages/cli/src/utils/nav-contribution-groups.ts b/packages/cli/src/utils/nav-contribution-groups.ts new file mode 100644 index 0000000000..c7f80f8a74 --- /dev/null +++ b/packages/cli/src/utils/nav-contribution-groups.ts @@ -0,0 +1,139 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * `os build`'s compile-time half of the #14553 ruling — the group id a + * `navigationContributions[]` entry names, checked where an AI author sees it + * first. + * + * ## Why the check lives here and not in `composeStacks` + * + * Maintainer ruling, 2026-09-02 (verbatim: 「同意」): "when the contributing + * package and the target app are composed into one artifact (`composeStacks`), + * `os build` checks the group id at compile time and reports the same + * diagnostic loudly there". The ruled location is the build STEP, and it is + * also the only one that can be right: `composeStacks` is a spec-level + * composition used by `os dev`, `os validate` and every embedder, whereas the + * question "did this author aim a contribution at a group that exists?" is a + * report, not a composition rule. Nothing here refuses — the runtime still + * relocates, deliberately (option A was weighed and not taken) — so a + * composition function is the wrong place to raise it. + * + * ## Why not an `@objectstack/lint` authoring rule + * + * `runAuthoringRules` hands a rule ONE stack (`run(stack, ctx)`), and every + * member of that table reads one. This question is cross-package by + * construction: the group id is declared by package A's app and named by + * package B's manifest, so the per-package walk in `compile.ts` sees neither + * half on its own and only the artifact does. Adding an artifact-aware member + * to a single-stack rule table is a larger platform change than the ruling + * asked for, and `compile.ts` already owns the artifact layer (`artifactPackages`, + * the capability preflight, the docs sweep). + * + * ## Why the predicate is imported and not written here + * + * {@link checkNavContributionGroups} is `@objectstack/objectql`'s, the same + * function `SchemaRegistry.applyNavContributions` folds through. A second copy + * of "does this group resolve?" would let the build call an aim fine that the + * runtime relocates — the exact silent divergence the card is about, one layer + * up. Loaded LAZILY, and only for a stack that actually declares a + * contribution: `os build`'s cold path should not pull the data engine in to + * judge two empty arrays. + */ + +import type { NavContributionGroupDiagnostic } from '@objectstack/objectql'; + +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 as `compile.ts` walks them. */ +export interface CompiledPackage { + readonly id: string; + readonly body: AnyRec; +} + +/** + * The apps and the contributions visible in ONE compilation unit. + * + * ⚠️ The top-level manifest is read only when the stack carries no `packages[]`. + * `composeStacks(…, { manifest: 'preserve' })` is additive: it flattens every + * collection to the top level AND still picks a singular `manifest` by the + * default `'last'` rule, so on an artifact the top-level + * `manifest.navigationContributions` is a COPY of one package's — reading both + * would report that package's mis-aim twice, once without a package id. + */ +export function collectNavGroupInputs( + parsed: AnyRec, + packages: readonly CompiledPackage[], +): { + apps: Array<{ name?: unknown; navigation?: unknown }>; + contributions: Array<{ app?: unknown; group?: unknown; items?: unknown; packageId?: string }>; +} { + const apps: Array<{ name?: unknown; navigation?: unknown }> = []; + const seenApps = new Set(); + const addApps = (list: unknown) => { + for (const entry of asArray(list)) { + const app = asRec(entry); + if (!app || typeof app.name !== 'string' || seenApps.has(app.name)) continue; + seenApps.add(app.name); + apps.push({ name: app.name, navigation: app.navigation }); + } + }; + + const contributions: Array<{ app?: unknown; group?: unknown; items?: unknown; packageId?: string }> = []; + const addContributions = (list: unknown, packageId?: string) => { + for (const entry of asArray(list)) { + const c = asRec(entry); + if (!c) continue; + contributions.push({ + app: c.app, + group: c.group, + items: c.items, + ...(packageId === undefined ? {} : { packageId }), + }); + } + }; + + // The flattened top level — present on every stack, artifact or not. + addApps(parsed.apps); + + if (packages.length > 0) { + for (const pkg of packages) { + // An assembled package body IS its own manifest (`compile.ts`' + // `packageBodyAsStack`), so both collections read off the top of it. + addApps(pkg.body.apps); + // `id` is the artifact's own package key (`manifest.id`, falling back to + // `name`), which is the string the runtime registers the contribution + // under — so the build names the package the same way the fold does. + addContributions(pkg.body.navigationContributions, pkg.id); + } + } else { + const manifest = asRec(parsed.manifest); + const packageId = typeof manifest?.id === 'string' + ? manifest.id + : (typeof manifest?.name === 'string' ? manifest.name : undefined); + addContributions(manifest?.navigationContributions, packageId); + } + + return { apps, contributions }; +} + +/** + * Every contribution in this compilation unit whose `group` names no group in + * the target app — the same finding, in the same words, the runtime fold + * raises when it relocates one. + * + * Returns `[]` without loading `@objectstack/objectql` when the stack declares + * no contributions at all, which is the overwhelming majority of builds. + */ +export async function findNavGroupDiagnostics( + parsed: AnyRec, + packages: readonly CompiledPackage[], +): Promise { + const { apps, contributions } = collectNavGroupInputs(parsed, packages); + if (contributions.length === 0 || apps.length === 0) return []; + const { checkNavContributionGroups } = await import('@objectstack/objectql/core'); + return checkNavContributionGroups(apps, contributions); +} diff --git a/packages/objectql/src/core.ts b/packages/objectql/src/core.ts index 394f5b9297..6b25f2bf14 100644 --- a/packages/objectql/src/core.ts +++ b/packages/objectql/src/core.ts @@ -24,6 +24,26 @@ export { } from './registry.js'; export type { ObjectContributor, SchemaRegistryOptions } from './registry.js'; +// [#14553] The navigation-contribution group diagnostic. Belongs on the LEAN +// entry as well as the barrel: it is a pure derivation over authored metadata +// with no imports of its own, so it costs this entry nothing and stays clear of +// the ADR-0076 D2 boundary ratchet. `os build` reaches it from here rather +// than through the batteries-included barrel, which would pull +// `@objectstack/metadata-protocol` into the compile path for a check that +// reads two arrays. +export { + NAV_CONTRIBUTION_GROUP_MISSING, + findNavGroup, + navContributionGroupDiagnostic, + formatNavContributionGroupDiagnostic, + checkNavContributionGroups, +} from './nav-contribution-diagnostics.js'; +export type { + NavContributionGroupDiagnostic, + NavGroupHostApp, + NavGroupContribution, +} from './nav-contribution-diagnostics.js'; + // Search-normalization companion column (#2486 — pinyin recall) export { SEARCH_COMPANION_FIELD, diff --git a/packages/objectql/src/index.ts b/packages/objectql/src/index.ts index 9a6dc785f3..fa6c3bbe71 100644 --- a/packages/objectql/src/index.ts +++ b/packages/objectql/src/index.ts @@ -66,6 +66,25 @@ export { } from './registry.js'; export type { InjectedColumnProvenance } from './registry.js'; +// [#14553] The navigation-contribution group diagnostic (ADR-0029 D7, +// ADR-0112 D6c). Exported because `os build` is the SECOND door that has to +// answer "does this group id resolve?" — over a composed artifact, at compile +// time, where an AI author sees the mis-aim first — and the whole point of the +// ruling is that the two doors report the same finding in the same words. A +// consumer asks these; it does not re-derive the walk. +export { + NAV_CONTRIBUTION_GROUP_MISSING, + findNavGroup, + navContributionGroupDiagnostic, + formatNavContributionGroupDiagnostic, + checkNavContributionGroups, +} from './nav-contribution-diagnostics.js'; +export type { + NavContributionGroupDiagnostic, + NavGroupHostApp, + NavGroupContribution, +} from './nav-contribution-diagnostics.js'; + // Search-normalization companion column (#2486 — pinyin recall). Shared by // the registry's compile-time provisioning seam, the engine's `$search` // expansion, and plugin-pinyin-search's populate hooks. diff --git a/packages/objectql/src/nav-contribution-diagnostics.ts b/packages/objectql/src/nav-contribution-diagnostics.ts new file mode 100644 index 0000000000..00ac19ae59 --- /dev/null +++ b/packages/objectql/src/nav-contribution-diagnostics.ts @@ -0,0 +1,213 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * ADR-0029 D7 / ADR-0130 — the diagnostic for a `navigationContributions[]` + * entry whose `group` names no group in the target app (#14553). + * + * ## What the platform does, and why this file exists + * + * `SchemaRegistry.applyNavContributions` RELOCATES such a contribution to the + * app's top level and carries on. That is deliberate and unchanged: the merge + * is a read-time fold precisely so registration order does not matter + * (`registerAppNavContribution` does not require the target app to exist yet), + * and a contribution into an OPTIONAL group must keep working. Refusing would + * trade both away — Option A of the card, not taken. + * + * What was wrong is that the relocation was INVISIBLE. The only trace was one + * `this.log(...)` line gated at `info`/`debug`, so a deployment running at + * `OS_REGISTRY_LOG=warn` — the level `OS_REGISTRY_LOG` exists to select, and + * the level this package's own vitest config pins — saw nothing at all while + * the information architecture silently changed underneath it. For a module + * split (hotcrm's 17-node navigation conversion) that is the WORSE of the two + * failures the card weighed: a dropped entry is missing and someone notices; a + * relocated one is present, passes a smoke test, and sits one level up from + * where its author put it. A typo'd group id is exactly what an AI author + * emits, so the failure had to become visible without becoming a refusal. + * + * Maintainer ruling, 2026-09-02 (verbatim: 「同意」) — option B: keep the + * relocation, make the trace a real diagnostic emitted at `warn`, and check the + * same condition at COMPILE time when both halves are composed into one + * artifact, where an AI author sees it first. + * + * ## One derivation, two halves — the reason this is a module and not two + * ## call sites + * + * The runtime half (`SchemaRegistry.applyNavContributions`) and the + * compile-time half (`os build`, `packages/cli/src/commands/compile.ts`) must + * answer "does this group id resolve?" the SAME way. Two copies of the + * predicate is the drift this card is about, one layer up: a build that says + * "fine" over a fold that relocates is indistinguishable, to the author, from + * the silence being fixed here. So {@link findNavGroup} is the single + * resolution — the registry's own private lookup now delegates to it — and + * {@link navContributionGroupDiagnostic} is the single message, formatted once + * by {@link formatNavContributionGroupDiagnostic} so both doors print the same + * line. + * + * ## Why the code is lowercase, and why no ledger entry + * + * ADR-0112 D6c, by name: "Diagnostics codes are not error codes." The ADR + * classifies a record that travels as payload of a success, describes an + * ARTIFACT rather than a request, carries a severity that can be `warning`, and + * is never routed to `error.code` — all four true here — as a separate + * vocabulary that "stays lowercase and out of the ledger". This entry is + * shaped after the ADR-0038 `BuildIssue` family that D6c names first + * (`{ severity, artifact, ref, code, message, fix }`, + * `metadata-protocol/src/build-probes.ts`), which is also what lets one stream + * carry the runtime and the build finding. + * + * ⛔ So: no `ERROR_CODE_LEDGER` registration and no `UNREGISTERED_CODE_SITES` + * row. `check:dispatcher-error-vocabulary` delegates a lowercase literal in an + * object-literal `code:` position to `check:error-code-casing`, which owns the + * D6/D6b/D6c discrimination through its `EXEMPT_FILES` list — this file is + * registered there with that reason, the same way `build-probes.ts` and + * `metadata-diagnostics.ts` are. + */ + +/** + * The diagnostics code for a contribution relocated past a missing group. + * + * Exported so consumers branch on the constant instead of re-spelling the + * literal — the D6c vocabulary is a contract even though it is not the error + * catalog's. + */ +export const NAV_CONTRIBUTION_GROUP_MISSING = 'nav_contribution_group_missing'; + +/** One relocated-contribution finding (ADR-0038 BuildIssue family; ADR-0112 D6c). */ +export interface NavContributionGroupDiagnostic { + /** Always {@link NAV_CONTRIBUTION_GROUP_MISSING}. */ + readonly code: typeof NAV_CONTRIBUTION_GROUP_MISSING; + /** Never `error`: the platform relocates and carries on — no refusal (option A not taken). */ + readonly severity: 'warning'; + /** The app whose navigation tree was changed. */ + readonly app: string; + /** The contributing package, when the registration carried one. */ + readonly packageId?: string; + /** The `group` id that resolved to no `type: 'group'` node in {@link app}. */ + readonly group: string; + /** Ids of the items moved to the app's top level, in contribution order. */ + readonly relocated: readonly string[]; + readonly message: string; + readonly fix: string; +} + +/** The shape this module needs from an app — its name and its nav tree. */ +export interface NavGroupHostApp { + readonly name?: unknown; + readonly navigation?: unknown; +} + +/** The shape this module needs from one contribution. */ +export interface NavGroupContribution { + readonly app?: unknown; + readonly group?: unknown; + readonly items?: unknown; + readonly packageId?: string; +} + +/** How an item with no `id` is named in {@link NavContributionGroupDiagnostic.relocated}. */ +const UNNAMED_ITEM = '(unnamed)'; + +/** + * Depth-first search for a `type: 'group'` nav item by id. + * + * ⚠️ BOTH conditions, and the pair is load-bearing: an `object`-type nav item + * that happens to share the id is not a container, so naming it is the same + * authoring error as naming nothing and must reach the same diagnostic. The + * registry's fold and `os build` call THIS function, so the two doors cannot + * disagree about what "the group exists" means. + */ +export function findNavGroup(items: unknown, groupId: string): Record | undefined { + if (!Array.isArray(items)) return undefined; + for (const item of items) { + if (!item || typeof item !== 'object') continue; + const node = item as Record; + if (node.id === groupId && node.type === 'group') return node; + const found = findNavGroup(node.children, groupId); + if (found) return found; + } + return undefined; +} + +/** The ids a contribution's items are reported under. */ +function relocatedIds(items: unknown): string[] { + if (!Array.isArray(items)) return []; + return items.map((item) => { + const id = (item as { id?: unknown } | null)?.id; + return typeof id === 'string' && id !== '' ? id : UNNAMED_ITEM; + }); +} + +/** + * Build the diagnostic for one contribution already known to name a missing + * group. Callers decide reachability (see {@link checkNavContributionGroups} + * and the registry fold); this only words the finding. + */ +export function navContributionGroupDiagnostic(input: { + app: string; + group: string; + packageId?: string; + items?: unknown; +}): NavContributionGroupDiagnostic { + const relocated = relocatedIds(input.items); + const who = input.packageId ?? '(unknown package)'; + const what = relocated.length === 1 ? '1 navigation item' : `${relocated.length} navigation items`; + return { + code: NAV_CONTRIBUTION_GROUP_MISSING, + severity: 'warning', + app: input.app, + ...(input.packageId === undefined ? {} : { packageId: input.packageId }), + group: input.group, + relocated, + message: + `Package "${who}" contributes ${what} [${relocated.join(', ')}] into group ` + + `"${input.group}" of app "${input.app}", but that app declares no such group — ` + + `the item(s) were RELOCATED to the app's top level. Nothing is lost and nothing ` + + `is refused, but the app's information architecture is not the one that was authored.`, + fix: + `Correct the group id to one the app declares as a \`type: "group"\` navigation node, ` + + `or declare "${input.group}" in app "${input.app}"'s own navigation.`, + }; +} + +/** One line carrying the whole finding — the text both doors print. */ +export function formatNavContributionGroupDiagnostic(d: NavContributionGroupDiagnostic): string { + return `[Registry] [${d.code}] ${d.message} Fix: ${d.fix}`; +} + +/** + * The COMPILE-TIME half: every contribution in one composed artifact whose + * `group` names no group in the target app. + * + * ⚠️ A contribution whose target app is NOT among `apps` yields nothing, and + * that silence is a decision rather than an oversight. A package may legally + * contribute into an app shipped by a DIFFERENT artifact installed separately — + * that is what makes the merge a read-time fold in the first place — so the + * only artifacts this can judge are the ones carrying both halves. Reporting + * the absent-app case here would refuse the supported cross-artifact case at + * build time, which is Option A wearing a warning's clothes. + */ +export function checkNavContributionGroups( + apps: readonly NavGroupHostApp[], + contributions: readonly NavGroupContribution[], +): NavContributionGroupDiagnostic[] { + const byName = new Map(); + for (const app of apps) { + if (typeof app?.name === 'string' && app.name !== '') byName.set(app.name, app); + } + + const found: NavContributionGroupDiagnostic[] = []; + for (const c of contributions) { + if (typeof c?.app !== 'string' || typeof c?.group !== 'string' || c.group === '') continue; + if (!Array.isArray(c.items) || c.items.length === 0) continue; + const target = byName.get(c.app); + if (!target) continue; + if (findNavGroup(target.navigation, c.group)) continue; + found.push(navContributionGroupDiagnostic({ + app: c.app, + group: c.group, + ...(c.packageId === undefined ? {} : { packageId: c.packageId }), + items: c.items, + })); + } + return found; +} diff --git a/packages/objectql/src/registry-nav-contribution-group-semantics.test.ts b/packages/objectql/src/registry-nav-contribution-group-semantics.test.ts index 6d75fcad01..d6887ba18b 100644 --- a/packages/objectql/src/registry-nav-contribution-group-semantics.test.ts +++ b/packages/objectql/src/registry-nav-contribution-group-semantics.test.ts @@ -24,24 +24,35 @@ * platform does a third thing: a contribution naming a missing group is * **relocated to the app's top level**. It does not vanish (so "silent drop", * the failure mode the card feared, is not what happens) and it does not fail - * (no throw, no `code`, no `console.warn`) — the only trace is ONE - * `console.log` line emitted by the registry's own `log()`, which is gated at - * `logLevel` `info`/`debug`. The shipped default IS `info`, so a default - * deployment prints it once at boot; a deployment running at `warn` — which is - * what `OS_REGISTRY_LOG` exists to select, and what this very package's - * `vitest.config.ts` selects — emits NOTHING at all. + * — no throw, and no refusal was ever added. * * For the conversion this is the worse half of the two outcomes the card * considered. A dropped entry is missing and someone notices; a relocated entry * is present, looks fine, and has silently changed the information architecture * — which is the exact property the 17-node conversion exists to preserve. A - * typo'd `group` therefore converts a nested entry into a top-level one and - * nothing red ever appears. + * typo'd `group` therefore converts a nested entry into a top-level one. * - * ⛔ This card is measurement only — no runtime behaviour is changed here. The - * pins below record what the platform does TODAY. If the PM rules that a - * missing group must refuse (or at minimum `console.warn`), the pin in - * `PROPOSITION 2` is what changes, in that card, deliberately. + * ## What #14553 changed, and what it deliberately did not + * + * Maintainer ruling, 2026-09-02 (verbatim: 「同意」) — option B. The + * RELOCATION is unchanged and its pin below is unchanged with it: the fold + * stays order-independent (`registerAppNavContribution` still does not require + * the target app to exist yet) and contributions into optional groups keep + * working. Option A — refuse at install, with the registration-order rule that + * implies — was weighed and NOT taken. + * + * What inverted is VISIBILITY. Until #14553 the only trace was one + * `console.log` line emitted by the registry's own `log()`, gated at + * `logLevel` `info`/`debug`: the shipped default IS `info`, so a default + * deployment printed it once at boot, while a deployment running at `warn` — + * which is what `OS_REGISTRY_LOG` exists to select, and what this very + * package's `vitest.config.ts` selects — emitted NOTHING while its information + * architecture changed. The trace is now a real diagnostic: an ADR-0038 + * BuildIssue-family record (ADR-0112 D6c — a diagnostics code, lowercase and + * out of the error ledger) carried on the app via `getAppNavDiagnostics` and + * announced through `console.warn`, so it survives `OS_REGISTRY_LOG=warn`. + * The `PROPOSITION 2 (visibility)` pin below is that inversion, asserted at + * three levels rather than assumed at one. * * ## The log assertions set `logLevel` explicitly, and must * @@ -56,6 +67,7 @@ import { describe, it, expect, afterEach } from 'vitest'; import { ObjectKernel } from '@objectstack/core'; import { SchemaRegistry } from './registry.js'; +import { NAV_CONTRIBUTION_GROUP_MISSING } from './nav-contribution-diagnostics.js'; import { ObjectQLPlugin } from './plugin.js'; import type { ObjectQL } from './engine.js'; @@ -174,7 +186,14 @@ describe('#14454 item 2 — `navigationContributions[].group` semantics across a // // ⚠️ This is the reading that matters for a 17-node conversion: the entry // is PRESENT, so no smoke test misses it, but it sits one level up from - // where the author put it — the information architecture changed, silently. + // where the author put it — the information architecture changed. + // + // ⛔ THIS PIN DOES NOT MOVE, and #14553 is the card that says so out loud. + // The ruling upgraded the TRACE (see the visibility pin) and changed the + // fold by not one line: refusing here would have introduced the + // registration-order constraint the read-time fold exists to avoid, and + // would have broken every package contributing into an OPTIONAL group. + // If this assertion ever goes red, the fold was changed — not the log. const nav = await mergedNav(contributorPackage('cpq', { group: 'group_that_does_not_exist' })); // Not dropped… @@ -183,7 +202,14 @@ describe('#14454 item 2 — `navigationContributions[].group` semantics across a expect(idsOf(groupOf(nav, 'sales_group')?.children)).toEqual(['nav_accounts']); }); - it('PROPOSITION 2 (visibility) — the ONLY trace is a `console.log` gated at `info`; nothing throws, nothing warns', () => { + it('PROPOSITION 2 (visibility) — [#14553 INVERTED] the relocation is a warn-level diagnostic, carried on the app; still nothing throws', () => { + // ⚠️ THIS PIN INVERTED IN #14553 and the direction is the whole point. + // Before: `logs` held one line at `info` and `warns` was EMPTY at every + // level, so the assertion `expect(atWarn.warns).toEqual([])` recorded the + // defect. It now asserts the opposite at the same level, over the same + // fixture, so the file reads as one continuous measurement rather than as + // a rewritten expectation. + // // Driven directly on a `SchemaRegistry` rather than through the kernel so // the level under test is the one this assertion names — the package's // vitest config pins `OS_REGISTRY_LOG=warn` for every registry constructed @@ -216,30 +242,163 @@ describe('#14454 item 2 — `navigationContributions[].group` semantics across a } finally { console.log = ol; console.warn = ow; console.error = oe; } - return { logs, warns, errors, threw, nav: app?.navigation ?? [] }; + return { + logs, warns, errors, threw, + nav: app?.navigation ?? [], + carried: registry.getAppNavDiagnostics('crm_app'), + }; }; - // At the SHIPPED default level, one line — and it is honest about what it did. - const atInfo = read('info'); - expect(atInfo.threw).toBeUndefined(); - expect(atInfo.logs).toHaveLength(1); - expect(atInfo.logs[0]).toContain('targets missing group "group_that_does_not_exist"'); - expect(atInfo.logs[0]).toContain('appending at top level'); - expect(atInfo.logs[0]).toContain('com.acme.crm.cpq'); - // ⚠️ Not a warning and not an error — the two channels an operator filters - // for. This is the visibility gap, pinned. - expect(atInfo.warns).toEqual([]); - expect(atInfo.errors).toEqual([]); - - // …and at `warn` — the level `OS_REGISTRY_LOG` selects, and the level this - // package's own test harness runs at — the relocation is COMPLETELY silent - // while still happening. + // ── The level the card is about ────────────────────────────────── + // `warn` is what `OS_REGISTRY_LOG` exists to select, what this package's + // own harness runs at, and what a quiet production boot runs at. It used to + // emit NOTHING while relocating; it now emits the diagnostic, on the + // channel an operator filters FOR. const atWarn = read('warn'); expect(atWarn.threw).toBeUndefined(); - expect(atWarn.logs).toEqual([]); - expect(atWarn.warns).toEqual([]); + expect(atWarn.warns).toHaveLength(1); + // Every fact the ruling named — the contributing package, the target app, + // the missing group id, and the relocated items — asserted individually, so + // a message rewrite that drops one goes red on that one rather than on a + // whole-string comparison nobody can read. + expect(atWarn.warns[0]).toContain(NAV_CONTRIBUTION_GROUP_MISSING); + expect(atWarn.warns[0]).toContain('com.acme.crm.cpq'); + expect(atWarn.warns[0]).toContain('crm_app'); + expect(atWarn.warns[0]).toContain('group_that_does_not_exist'); + expect(atWarn.warns[0]).toContain('nav_cpq'); + expect(atWarn.warns[0]).toContain('RELOCATED'); + // Still not an ERROR and still no throw: a diagnostic was added, a refusal + // was not. Option A stays untaken, and this is the assertion that says so. expect(atWarn.errors).toEqual([]); + expect(atWarn.logs).toEqual([]); + // …and the items are exactly where the unchanged fold puts them. expect(idsOf(atWarn.nav)).toEqual(['sales_group', 'nav_cpq']); + + // ── Carried on the app, not only printed ───────────────────────── + // The ruling asked for a diagnostic the app CARRIES, so `os doctor`, a boot + // report or a test can ask the app what happened to it instead of scraping + // a log line. + expect(atWarn.carried).toHaveLength(1); + expect(atWarn.carried[0]).toMatchObject({ + code: NAV_CONTRIBUTION_GROUP_MISSING, + severity: 'warning', + app: 'crm_app', + packageId: 'com.acme.crm.cpq', + group: 'group_that_does_not_exist', + relocated: ['nav_cpq'], + }); + + // ── The shipped default behaves the same ───────────────────────── + // `info` is BELOW `warn` on the ladder, so a default deployment sees the + // diagnostic too — once, not once per channel. + const atInfo = read('info'); + expect(atInfo.threw).toBeUndefined(); + expect(atInfo.warns).toHaveLength(1); + expect(atInfo.warns[0]).toBe(atWarn.warns[0]); + expect(atInfo.errors).toEqual([]); + + // ── And silence is still available, without losing the record ──── + // A deployment that asked for silence gets it. The diagnostic is still + // CARRIED, which is what separates "quiet" from "not measured": muting the + // log must not destroy the app's own verdict. + const atSilent = read('silent'); + expect(atSilent.logs).toEqual([]); + expect(atSilent.warns).toEqual([]); + expect(atSilent.errors).toEqual([]); + expect(atSilent.carried).toHaveLength(1); + expect(idsOf(atSilent.nav)).toEqual(['sales_group', 'nav_cpq']); + }); + + it('[#14553] the diagnostic is emitted ONCE per mis-aim, however many times the app is read', () => { + // The merge is a READ-time fold — it runs on every `getApp` — so a + // diagnostic emitted per fold would print once per request. That is the + // other half of the #12015 discipline: a line that fires on every read is + // as unreadable as one that never fires, and it would make this diagnostic + // the thing operators filter OUT. + // + // The de-duplication is per REGISTRY, not per process. A module-level memo + // would make the second registry in a process silent — which is exactly the + // shape the pin above reads (three fresh registries, same mis-aim each). + const registry = new SchemaRegistry({ multiTenant: false, collisionPolicy: 'error' }); + registry.logLevel = 'warn'; + registry.registerApp( + { name: 'crm_app', label: 'CRM', navigation: [{ id: 'sales_group', type: 'group', label: 'Sales', children: [] }] }, + 'com.acme.crm', + ); + registry.registerAppNavContribution( + { app: 'crm_app', group: 'group_that_does_not_exist', items: [{ id: 'nav_cpq', type: 'object', objectName: 'crm_cpq', label: 'cpq' }] }, + 'com.acme.crm.cpq', + ); + + const warns: string[] = []; + const ow = console.warn; + const capture = (...a: unknown[]) => { warns.push(a.map(String).join(' ')); }; + + console.warn = capture; + try { + registry.getApp('crm_app'); + registry.getApp('crm_app'); + registry.getApp('crm_app'); + } finally { + console.warn = ow; + } + expect(warns).toHaveLength(1); + // …and the carried record is deduplicated with it, so a long-running + // process does not accumulate one entry per read of the same app. + expect(registry.getAppNavDiagnostics('crm_app')).toHaveLength(1); + + // A DIFFERENT mis-aim on the same app is a different finding and speaks — + // the floor under the de-duplication, without which "once" could be + // satisfied by a memo that silences everything after the first line. + registry.registerAppNavContribution( + { app: 'crm_app', group: 'another_missing_group', items: [{ id: 'nav_quote', type: 'object', objectName: 'crm_quote', label: 'quote' }] }, + 'com.acme.crm.quote', + ); + console.warn = capture; + try { + registry.getApp('crm_app'); + } finally { + console.warn = ow; + } + expect(warns).toHaveLength(2); + expect(registry.getAppNavDiagnostics('crm_app')).toHaveLength(2); + }); + + it('[#14553] a contribution that RESOLVES raises no diagnostic — what is reported is the mis-aim, not the mechanism', () => { + // The floor under every assertion above. Without it, a diagnostic that + // fired on EVERY contribution would satisfy all of them, and the platform + // would have traded a silent relocation for a warning nobody can act on. + const registry = new SchemaRegistry({ multiTenant: false, collisionPolicy: 'error' }); + registry.logLevel = 'warn'; + registry.registerApp( + { name: 'crm_app', label: 'CRM', navigation: [{ id: 'sales_group', type: 'group', label: 'Sales', children: [] }] }, + 'com.acme.crm', + ); + registry.registerAppNavContribution( + { app: 'crm_app', group: 'sales_group', items: [{ id: 'nav_cpq', type: 'object', objectName: 'crm_cpq', label: 'cpq' }] }, + 'com.acme.crm.cpq', + ); + // …and a group-LESS contribution, which lands at the top level BY DESIGN + // and must never be reported as a mis-aim. + registry.registerAppNavContribution( + { app: 'crm_app', items: [{ id: 'nav_help', type: 'object', objectName: 'crm_help', label: 'help' }] }, + 'com.acme.crm.help', + ); + + const warns: string[] = []; + const ow = console.warn; + console.warn = (...a: unknown[]) => { warns.push(a.map(String).join(' ')); }; + let nav: NavItem[] = []; + try { + nav = (registry.getApp('crm_app') as { navigation?: NavItem[] }).navigation ?? []; + } finally { + console.warn = ow; + } + + expect(warns).toEqual([]); + expect(registry.getAppNavDiagnostics('crm_app')).toEqual([]); + expect(idsOf(groupOf(nav, 'sales_group')?.children)).toEqual(['nav_cpq']); + expect(idsOf(nav)).toEqual(['sales_group', 'nav_help']); }); it('PROPOSITION 2 (authoring door) — the authoring gate cannot catch it either: `group` reaches no cross-reference check', async () => { @@ -250,6 +409,14 @@ describe('#14454 item 2 — `navigationContributions[].group` semantics across a // observable consequence is that a typo'd group id survives BOTH doors: it // installs, and it relocates. // + // ⛔ STILL TRUE AFTER #14553, and that is the point of keeping it. The + // ruling put the diagnostic on the FOLD (where the relocation happens) and + // on `os build` (where an author sees it first) — not on registration, + // which by design does not know whether the target app exists yet, let + // alone which groups it declares. So the install door is as quiet as it + // ever was, deliberately, and this pin is what would notice a refusal + // creeping in here. + // // Asserted at the install door, where the consequence is observable: the // contribution is recorded verbatim, group id and all, with no diagnostic. const kernel = new ObjectKernel({ logger: { level: 'silent' }, gracefulShutdown: false }); diff --git a/packages/objectql/src/registry.ts b/packages/objectql/src/registry.ts index 6f4c2b2f08..c0699cf0d8 100644 --- a/packages/objectql/src/registry.ts +++ b/packages/objectql/src/registry.ts @@ -42,6 +42,17 @@ import { applyProtection } from '@objectstack/spec/shared'; // same string the artifact loader ordered them by, or a co-owner would be // admitted — or refused — under a key nothing else in the path uses. import { artifactPackageId } from '@objectstack/core'; +// [#14553] The ONE resolution of "does this nav group id exist?" and the ONE +// wording of the diagnostic when it does not. `os build` calls the same two +// (`checkNavContributionGroups`), so the compile-time door and this read-time +// fold cannot disagree about what a missing group is — the drift that would +// make the build's silence indistinguishable from the silence being fixed. +import { + findNavGroup as resolveNavGroup, + navContributionGroupDiagnostic, + formatNavContributionGroupDiagnostic, + type NavContributionGroupDiagnostic, +} from './nav-contribution-diagnostics.js'; /** * Reserved namespaces that do not get FQN prefix applied. @@ -1644,6 +1655,23 @@ export class SchemaRegistry { console.log(msg); } + /** + * [#14553] A diagnostic an operator filters FOR — emitted at every level down + * to `warn`, silent only where the deployment has asked for silence + * (`error`, `silent`). + * + * The level ladder is `debug < info < warn < error < silent`, so this is the + * mirror of {@link log}: that one speaks only ABOVE `warn` and is therefore + * the wrong channel for anything a `warn`-level deployment must not miss. + * `OS_REGISTRY_LOG=warn` is what the objectql vitest config selects and what + * a quiet production boot selects, and a relocation nobody can see there was + * the defect this method exists to end. + */ + private warn(msg: string): void { + if (this._logLevel === 'silent' || this._logLevel === 'error') return; + console.warn(msg); + } + /** * Debug-only diagnostic: emitted solely when `logLevel === 'debug'`, so it * stays out of the default (`'info'`) boot log. Use for expected-but-noisy @@ -4226,10 +4254,14 @@ export class SchemaRegistry { if (!Array.isArray(group.children)) group.children = []; group.children.push(...c.items); } else { - this.log( - `[Registry] Navigation contribution from "${c.packageId ?? '(unknown)'}" targets ` + - `missing group "${c.group}" in app "${app.name}" — appending at top level.`, - ); + // [#14553] The relocation itself is UNCHANGED — the fold stays + // order-independent and contributions into optional groups keep + // working (option A, a refusal plus a registration-order rule, was + // weighed and not taken). What changed is that it is no longer + // silent: this used to be one `this.log(...)` gated at `info`, so a + // deployment at `OS_REGISTRY_LOG=warn` watched its information + // architecture change with nothing in the log at all. + this.recordNavGroupMiss(String(app.name), c); nav.push(...c.items); } } else { @@ -4239,16 +4271,77 @@ export class SchemaRegistry { return cloned; } - /** Depth-first search for a `type: 'group'` nav item by id. */ + /** + * Depth-first search for a `type: 'group'` nav item by id. + * + * [#14553] Delegates rather than implements: `os build` asks the identical + * question at compile time over a composed artifact, and two copies of this + * walk is exactly the drift that would let the build call an authoring error + * fine while this fold relocates it. + */ private findNavGroup(items: any[], groupId: string): any | undefined { - for (const item of items) { - if (item && item.id === groupId && item.type === 'group') return item; - if (item && Array.isArray(item.children)) { - const found = this.findNavGroup(item.children, groupId); - if (found) return found; - } - } - return undefined; + return resolveNavGroup(items, groupId); + } + + // ========================================== + // Navigation-contribution diagnostics (#14553) + // ========================================== + + /** + * Diagnostics raised by the fold, keyed `app` → `packageId|group` → entry. + * + * ⚠️ PER REGISTRY and DE-DUPLICATED, both deliberate. `applyNavContributions` + * runs on EVERY read of an app (that is what makes it a fold rather than a + * mutation), so an undeduplicated list would grow without bound and an + * undeduplicated `console.warn` would print once per request — which is the + * #12015 discipline's other failure: a diagnostic that fires on every read is + * as unreadable as one that never fires. Keyed on the triple the author has + * to act on, the line is emitted once per process per distinct mis-aim. + * + * ⛔ Not a module-level `Set` (the shape `warnStrippedLegacyApiMethods` + * uses): registries are constructed per kernel and per test, and a + * process-global memo would make the SECOND registry that hits the same + * mis-aim silent — including, measurably, the pin that reads this + * diagnostic at two log levels in a row. + */ + private navGroupDiagnostics = new Map>(); + + /** + * Record — and, the first time, announce — one relocated contribution. + * + * The entry is kept so a caller that is not reading the log + * ({@link getAppNavDiagnostics}: `os doctor`, a boot report, a test) can ask + * the app what happened to it. + */ + private recordNavGroupMiss( + appName: string, + c: { packageId?: string; group?: string; items: any[] }, + ): void { + const diagnostic = navContributionGroupDiagnostic({ + app: appName, + group: String(c.group), + ...(c.packageId === undefined ? {} : { packageId: c.packageId }), + items: c.items, + }); + const perApp = this.navGroupDiagnostics.get(appName) ?? new Map(); + const key = `${c.packageId ?? ''}|${c.group}`; + if (perApp.has(key)) return; + perApp.set(key, diagnostic); + this.navGroupDiagnostics.set(appName, perApp); + this.warn(formatNavContributionGroupDiagnostic(diagnostic)); + } + + /** + * Navigation-contribution diagnostics raised for `appName` so far, in the + * order they were first raised (empty when the app's contributions all + * resolved, or when nothing has read the app yet). + * + * ⚠️ Populated BY the read-time fold, so it answers for the reads that have + * happened — call `getApp(appName)` first if you need the app's own verdict + * rather than the process's history. + */ + getAppNavDiagnostics(appName: string): NavContributionGroupDiagnostic[] { + return [...(this.navGroupDiagnostics.get(appName)?.values() ?? [])]; } // ========================================== From 74e08e2bd6fe93bb4b98742a1afd3350be462d78 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 3 Sep 2026 09:35:41 +0000 Subject: [PATCH 2/8] test(cli): pin the nav-group check through the command's own parse chain Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01UHvF5hyiZjnCyExFnfQB8m --- .../src/utils/nav-contribution-groups.test.ts | 45 ++++++++++++++++++- 1 file changed, 44 insertions(+), 1 deletion(-) diff --git a/packages/cli/src/utils/nav-contribution-groups.test.ts b/packages/cli/src/utils/nav-contribution-groups.test.ts index fc04b7dd1f..ea841d26c6 100644 --- a/packages/cli/src/utils/nav-contribution-groups.test.ts +++ b/packages/cli/src/utils/nav-contribution-groups.test.ts @@ -40,7 +40,7 @@ */ import { describe, it, expect, beforeAll } from 'vitest'; -import { composeStacks } from '@objectstack/spec'; +import { composeStacks, normalizeStackInput, ObjectStackDefinitionSchema } from '@objectstack/spec'; import { collectNavGroupInputs, findNavGroupDiagnostics } from './nav-contribution-groups.js'; type AnyRec = Record; @@ -104,6 +104,30 @@ const ordersStack = (group: string) => ({ const artifact = (group: string): AnyRec => composeStacks([ordersStack(group), coreStack()], { manifest: 'preserve' }) as unknown as AnyRec; +/** + * The composed artifact as `compile.ts` actually hands it to this check — + * through the SAME `normalizeStackInput` + `ObjectStackDefinitionSchema` + * parse the command runs, and `result.data` is the object it passes. + * + * ⚠️ Load-bearing, and the one thing a raw `composeStacks` reading cannot + * establish. The check walks `parsed.packages[].manifest`, and if the parse + * reshaped, renamed or dropped any part of that path the derivation would + * silently see an empty artifact — reporting nothing, which is + * indistinguishable from a clean stack. This is asserted rather than assumed + * because the alternative was a 57-package build of the CLI's dependency + * closure to spawn one `os build`. + */ +const parsedArtifact = (group: string): AnyRec => { + const normalized = normalizeStackInput(artifact(group) as Record, { + 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; +}; + /** The artifact's packages, in the `{ index, id, body }` shape `compile.ts` walks. */ const packagesOf = (parsed: AnyRec) => ((parsed.packages ?? []) as Array<{ manifest?: AnyRec }>).map((entry, index) => { @@ -217,6 +241,25 @@ describe('#14553 — `os build` checks `navigationContributions[].group` across expect(found[0]).toMatchObject({ app: APP, group: 'sales_grp', packageId: 'com.example.single' }); }); + it('survives the command\'s own parse — what `compile.ts` passes in is what this reads', async () => { + // The end-to-end statement this file can make without spawning the CLI: + // run the fixture through the real `normalizeStackInput` + + // `ObjectStackDefinitionSchema` chain `os build` runs, hand `result.data` + // to the derivation exactly as the command does, and require the SAME + // finding the raw composed object produces. + // + // Both directions matter. The typo'd leg proves the parse does not hide the + // path this check walks; the clean leg proves it does not invent findings. + const typod = parsedArtifact('sales_grp'); + const found = await findNavGroupDiagnostics(typod, packagesOf(typod)); + expect(found).toHaveLength(1); + expect(found[0]).toMatchObject({ app: APP, packageId: ORDERS_ID, group: 'sales_grp', relocated: ['nav_orders'] }); + + const clean = parsedArtifact(GROUP); + expect(packagesOf(clean).map((pkg) => pkg.id).sort()).toEqual([CORE_ID, ORDERS_ID]); + expect(await findNavGroupDiagnostics(clean, packagesOf(clean))).toEqual([]); + }); + it('naming an id that exists but is not a `type: "group"` node reaches the same diagnostic', async () => { // `findNavGroup` requires BOTH `id` and `type === 'group'`, so pointing at // the app's `nav_accounts` object entry is the same authoring error as From 49fd7d30e664a7f14d42122eac49a47c59c390ef Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 3 Sep 2026 09:58:58 +0000 Subject: [PATCH 3/8] chore: classify the new diagnostic code, changeset, and gate repairs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - `packages/runtime/src/dispatcher-error-vocabulary.ts`: classify `nav_contribution_group_missing` as `foreign-vocabulary` / door `none`. ADR-0112 D6c on all four of its tests — payload of a success, describes an artifact, severity `warning`, never routed to `error.code` — so it is lowercase and stays OUT of the error-code ledger. The gate reports it here rather than delegating to `check:error-code-casing` because the constant is referenced (`objlitconst`), not quoted at the stamp. - `content/docs/permissions/system-context.mdx`: line rot repaired by `check-system-context-census --fix` after the registry edit shifted anchors. - `packages/cli/src/utils/nav-contribution-groups.test.ts`: module-top side-effect load of the dist-resolved dependency, per `check:test-source-alias` — a first load inside a clocked window is the measured cliff that gate exists to stop. - changeset: objectql + cli patch. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01UHvF5hyiZjnCyExFnfQB8m --- ...ontribution-group-relocation-diagnostic.md | 50 +++++++++++++++++++ content/docs/permissions/system-context.mdx | 2 +- .../src/utils/nav-contribution-groups.test.ts | 30 +++++------ .../src/dispatcher-error-vocabulary.ts | 21 ++++++++ 4 files changed, 88 insertions(+), 15 deletions(-) create mode 100644 .changeset/nav-contribution-group-relocation-diagnostic.md diff --git a/.changeset/nav-contribution-group-relocation-diagnostic.md b/.changeset/nav-contribution-group-relocation-diagnostic.md new file mode 100644 index 0000000000..18d68d0351 --- /dev/null +++ b/.changeset/nav-contribution-group-relocation-diagnostic.md @@ -0,0 +1,50 @@ +--- +"@objectstack/objectql": patch +"@objectstack/cli": patch +--- + +fix(objectql,cli): a navigation contribution relocated past a missing group now says so, at `warn` and at build time + +A package that injects navigation into another package's app names the target +container by id (`navigationContributions[].group`). When that id matches no +`type: "group"` node in the target app, `SchemaRegistry.applyNavContributions` +appends the items at the app's **top level** and continues. + +That relocation is unchanged, deliberately. The merge is a read-time fold +precisely so registration order does not matter — `registerAppNavContribution` +does not require the target app to exist yet — and a package contributing into +an *optional* group has to keep working. Refusing would trade both away. + +What changes is that it is no longer invisible. The only trace used to be one +log line gated at `info`/`debug`, so a deployment running at +`OS_REGISTRY_LOG=warn` watched its information architecture change in complete +silence: a typo'd group id — exactly what an AI author emits — turned a nested +menu entry into a top-level one, and because the entry was still *present*, no +smoke test noticed. That is worse than a dropped entry, which someone notices. + +The trace is now a real diagnostic naming the contributing package, the target +app, the missing group id and the relocated items: + +- **At runtime.** An ADR-0038 `BuildIssue`-family record (ADR-0112 D6c — a + diagnostics code, lowercase and out of the error ledger) is carried on the + app and reachable as `registry.getAppNavDiagnostics(appName)`, and announced + through `console.warn`, so it survives `OS_REGISTRY_LOG=warn` and reaches + `os doctor` / boot output. Emitted once per registry per distinct mis-aim: + the fold runs on every read of the app, and a line printed per request is as + unreadable as one never printed. A deployment that asks for `silent` still + gets silence, and still keeps the record. +- **At build time.** `os build` answers the same question over a composed + artifact, through the same predicate, and reports the same finding where an + author sees it first — in the text output and in `--json` under + `navigationGroupDiagnostics`. A contribution aimed at an app no package in + the artifact ships is not reported: contributing into an app another artifact + installs is the supported case, and is why the merge is a fold. + +**Nothing is refused.** No new failure, no ordering constraint, no change to +what installs or to what `os build` accepts — a diagnostic was added and a +refusal was not. + +`examples/app-multi-package` now demonstrates the mechanism it was missing: the +App package publishes a `sales_group` container and the Orders module +contributes its nav entry into it, which is what a module split converts an +app's own navigation into. diff --git a/content/docs/permissions/system-context.mdx b/content/docs/permissions/system-context.mdx index 6990eb7383..4361fd2f8a 100644 --- a/content/docs/permissions/system-context.mdx +++ b/content/docs/permissions/system-context.mdx @@ -235,7 +235,7 @@ should recognise it instead of re-deriving it. rule-materialised grant that the next reconcile silently restores. 5. **`applySystemFields` does not read this flag.** It is named as if it did. - `packages/objectql/src/registry.ts:464` is **schema-side column + `packages/objectql/src/registry.ts:475` is **schema-side column provisioning** — which columns an object carries — and consumes `ExecutionContext.isSystem` zero times. The write-time ownership behaviour people attribute to it is row 2, in `plugin-security`. diff --git a/packages/cli/src/utils/nav-contribution-groups.test.ts b/packages/cli/src/utils/nav-contribution-groups.test.ts index ea841d26c6..25f7edfa7d 100644 --- a/packages/cli/src/utils/nav-contribution-groups.test.ts +++ b/packages/cli/src/utils/nav-contribution-groups.test.ts @@ -39,7 +39,20 @@ * keep in sync, and a divergence in it would read as a finding. */ -import { describe, it, expect, beforeAll } from 'vitest'; +// [#7668 family, `check:test-source-alias`] A MODULE-TOP side-effect load of +// the dependency `findNavGroupDiagnostics` imports dynamically. This package +// resolves `@objectstack/objectql/core` through its `dist/`, so the first call +// transforms that whole module graph — measured here at over vitest's 5s +// default, inside a test body, which is a CLOCKED window. Paying it during +// COLLECTION (which vitest clocks against nothing) is the convention: clocked +// windows measure behaviour, never loading. ⛔ Do not "fix" a timeout here by +// widening it — that relocates the cliff to the next heavier shard. +// +// 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 { describe, it, expect } from 'vitest'; import { composeStacks, normalizeStackInput, ObjectStackDefinitionSchema } from '@objectstack/spec'; import { collectNavGroupInputs, findNavGroupDiagnostics } from './nav-contribution-groups.js'; @@ -89,7 +102,7 @@ const ordersStack = (group: string) => ({ navigationContributions: [{ app: APP, group, - items: [{ id: 'nav_orders', type: 'object', objectName: 'crm_order', label: 'Orders' }], + items: [{ id: 'nav_orders', type: 'object' as const, objectName: 'crm_order', label: 'Orders' }], }], }, objects: [{ @@ -139,17 +152,6 @@ const packagesOf = (parsed: AnyRec) => }); describe('#14553 — `os build` checks `navigationContributions[].group` across one composed artifact', () => { - // `findNavGroupDiagnostics` loads `@objectstack/objectql/core` LAZILY, and - // that first load costs several seconds — measured here, over vitest's 5s - // default, which is what surfaced it. That cost is the reason the import is - // lazy in production too: `os build`'s cold path must not pull the data - // engine in to judge a stack that declares no contributions at all. Warmed - // once, with its own budget, so the per-test durations below report what the - // derivation costs rather than what the module loader does. - beforeAll(async () => { - await import('@objectstack/objectql/core'); - }, 60_000); - it('the fixture really is a two-package artifact — the floor under every reading below', async () => { // Without this, a composition change that stopped emitting `packages[]` // would make every assertion in this file vacuously true: the derivation @@ -231,7 +233,7 @@ describe('#14553 — `os build` checks `navigationContributions[].group` across navigationContributions: [{ app: APP, group: 'sales_grp', - items: [{ id: 'nav_orders', type: 'object', objectName: 'crm_order', label: 'Orders' }], + items: [{ id: 'nav_orders', type: 'object' as const, objectName: 'crm_order', label: 'Orders' }], }], }, apps: coreStack().apps, diff --git a/packages/runtime/src/dispatcher-error-vocabulary.ts b/packages/runtime/src/dispatcher-error-vocabulary.ts index 3690a65db6..318570e3f7 100644 --- a/packages/runtime/src/dispatcher-error-vocabulary.ts +++ b/packages/runtime/src/dispatcher-error-vocabulary.ts @@ -309,6 +309,27 @@ export const UNREGISTERED_CODE_SITES: readonly UnregisteredCodeSite[] = [ // (no source site — the producer is tenant code; see SANDBOX_AUTHORED_LIMB) // ── foreign vocabularies: spelled `code`, not an ADR-0112 error.code ──── + { + code: 'nav_contribution_group_missing', + file: 'packages/objectql/src/nav-contribution-diagnostics.ts', + shape: 'objlitconst', + door: 'none', + verdict: 'foreign-vocabulary', + why: + 'ADR-0112 D6c by name, on all four of its tests: it ships as PAYLOAD of a success (carried on ' + + 'the served app, and on `os build --json` under `navigationGroupDiagnostics`, from runs that ' + + 'exit 0), it describes an ARTIFACT rather than a request (which app, which package, which ' + + 'group id), its severity is `warning` and cannot be anything else, and no path routes it to ' + + '`error.code` — the producer is a read-time FOLD (`applyNavContributions`) and a compile ' + + 'step, neither of which throws. #14553 added the diagnostic and deliberately added NO ' + + 'refusal: the contribution is still relocated to the app\'s top level and still installs, so ' + + 'there is no failing request for a catalog to govern. Hence lowercase and out of the ledger, ' + + 'exactly as D6c prescribes for `metadata-diagnostics.ts` and `build-probes.ts`, whose ' + + '`{ severity, artifact, ref, code, message, fix }` family this record is shaped after. ' + + 'Reported here rather than delegated to `check:error-code-casing` because the constant is ' + + 'REFERENCED (`objlitconst`), not quoted at the stamp — that gate\'s lowercase patterns all ' + + 'need a literal beside the token, so this position is this gate\'s to classify.', + }, { code: 'MODULE_NOT_FOUND', file: 'packages/types/src/node.ts', From 2ab7f869583e378f0d92df93657cc33da9bda4a4 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 3 Sep 2026 10:11:10 +0000 Subject: [PATCH 4/8] chore(runtime): keep the tracker id out of the classification row's prose MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `check:doc-authoring` — a runtime string reaches authors, operators and generated surfaces, none of whom can resolve `#NNNN`. The anchor moves to an adjacent comment, where the reader who can resolve it is already looking. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01UHvF5hyiZjnCyExFnfQB8m --- packages/runtime/src/dispatcher-error-vocabulary.ts | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/packages/runtime/src/dispatcher-error-vocabulary.ts b/packages/runtime/src/dispatcher-error-vocabulary.ts index 318570e3f7..fc98977ab9 100644 --- a/packages/runtime/src/dispatcher-error-vocabulary.ts +++ b/packages/runtime/src/dispatcher-error-vocabulary.ts @@ -309,6 +309,7 @@ export const UNREGISTERED_CODE_SITES: readonly UnregisteredCodeSite[] = [ // (no source site — the producer is tenant code; see SANDBOX_AUTHORED_LIMB) // ── foreign vocabularies: spelled `code`, not an ADR-0112 error.code ──── + // [#14553] The navigation-contribution relocation diagnostic. { code: 'nav_contribution_group_missing', file: 'packages/objectql/src/nav-contribution-diagnostics.ts', @@ -321,9 +322,10 @@ export const UNREGISTERED_CODE_SITES: readonly UnregisteredCodeSite[] = [ 'exit 0), it describes an ARTIFACT rather than a request (which app, which package, which ' + 'group id), its severity is `warning` and cannot be anything else, and no path routes it to ' + '`error.code` — the producer is a read-time FOLD (`applyNavContributions`) and a compile ' + - 'step, neither of which throws. #14553 added the diagnostic and deliberately added NO ' + - 'refusal: the contribution is still relocated to the app\'s top level and still installs, so ' + - 'there is no failing request for a catalog to govern. Hence lowercase and out of the ledger, ' + + 'step, neither of which throws. The card that added it added the diagnostic and ' + + 'deliberately added NO refusal: the contribution is still relocated to the app\'s top ' + + 'level and still installs, so there is no failing request for a catalog to govern. Hence ' + + 'lowercase and out of the ledger, ' + 'exactly as D6c prescribes for `metadata-diagnostics.ts` and `build-probes.ts`, whose ' + '`{ severity, artifact, ref, code, message, fix }` family this record is shaped after. ' + 'Reported here rather than delegated to `check:error-code-casing` because the constant is ' + From e854bc8775323110c4d992afda2c562df023255a Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 3 Sep 2026 10:29:35 +0000 Subject: [PATCH 5/8] docs(ui): state what happens when a nav contribution names an anchor that is not there MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Setup app page explains the shell/anchor mechanism — the shell publishes empty group anchors and capability plugins contribute into them — but stopped short of the failure the anchor id makes possible. A contributor cannot see the shell's ids at authoring time, so a typo is undetectable from its own source, and the platform relocates rather than refusing: the menu renders, a smoke test passes, and the entry has moved one level up. Documents the rule and the diagnostic that now reports it: the `warn`-level `nav_contribution_group_missing`, the per-app `getAppNavDiagnostics` reader, and the `os build --json` `navigationGroupDiagnostics` key — following the `bodyExtractionWarnings` precedent, which documents a build-only JSON key on the page that owns the behaviour rather than in a CLI schema dump. No hand-written page was falsified by the code change; this fills a gap rather than correcting an error. The `packages/spec` describe() for the key has the same gap and is filed separately — that string is embedded in ~14 generated artifacts, so it belongs in its own PR. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01UHvF5hyiZjnCyExFnfQB8m --- content/docs/ui/setup-app.mdx | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/content/docs/ui/setup-app.mdx b/content/docs/ui/setup-app.mdx index ca6baf5dbd..1671242062 100644 --- a/content/docs/ui/setup-app.mdx +++ b/content/docs/ui/setup-app.mdx @@ -17,6 +17,35 @@ group id + priority on read, so the rendered menu reflects exactly which capability plugins are loaded — a disabled capability contributes nothing and its slot stays empty. +### When a contribution names an anchor that is not there + +Aiming at a `group` id the target app does not declare is **not** refused and +the entry is **not** dropped: the items are appended at the app's **top level** +and the merge continues. That is deliberate — the merge is a read-time fold +precisely so registration order does not matter (a contributor may register +before the app it aims at), and a contribution into an *optional* anchor has to +keep working when the plugin owning that anchor is not loaded. + +It is, however, **loud**. The relocation emits a +`nav_contribution_group_missing` diagnostic at `warn` — naming the contributing +package, the target app, the missing group id and the relocated items — so it +survives `OS_REGISTRY_LOG=warn` and appears in boot output. The same finding is +carried on the app itself, readable as +`registry.getAppNavDiagnostics(appName)`, and it is raised once per distinct +mis-aim rather than once per read of the app. + +`os build` answers the same question at compile time whenever the contributing +package and the target app are composed into one artifact, printing the finding +and carrying it in `--json` under `navigationGroupDiagnostics` (an empty array +when every contribution resolved). It **reports** there; it does not fail the +build. + +⚠️ The anchor id is the whole contract between a shell and its contributors, +and a contributor cannot see the shell's ids at authoring time. A typo +therefore produces a menu that renders, passes a smoke test, and has silently +moved the entry one level up — which is why the diagnostic exists rather than a +refusal. + The app itself ships from the dedicated `@objectstack/setup` package (package id `com.objectstack.setup`), which registers it at runtime. Per ADR-0048 (one app per package), `/apps/setup` — resolvable as From d47ad8961dc64aa099ad904a6fa34ae8f1b72419 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 3 Sep 2026 10:49:22 +0000 Subject: [PATCH 6/8] docs(ui): home the mis-aimed-anchor section under Navigation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It landed mid-intro on the first pass, which orphaned the intro's closing paragraph under an H3 and put that H3 ahead of the page's first H2. It now sits at the end of `## Navigation`, directly after the anchor table — where a reader has just learned the anchor ids and would ask what happens if one is wrong. Content unchanged. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01UHvF5hyiZjnCyExFnfQB8m --- content/docs/ui/setup-app.mdx | 58 +++++++++++++++++------------------ 1 file changed, 29 insertions(+), 29 deletions(-) diff --git a/content/docs/ui/setup-app.mdx b/content/docs/ui/setup-app.mdx index 1671242062..1f85efcea8 100644 --- a/content/docs/ui/setup-app.mdx +++ b/content/docs/ui/setup-app.mdx @@ -17,35 +17,6 @@ group id + priority on read, so the rendered menu reflects exactly which capability plugins are loaded — a disabled capability contributes nothing and its slot stays empty. -### When a contribution names an anchor that is not there - -Aiming at a `group` id the target app does not declare is **not** refused and -the entry is **not** dropped: the items are appended at the app's **top level** -and the merge continues. That is deliberate — the merge is a read-time fold -precisely so registration order does not matter (a contributor may register -before the app it aims at), and a contribution into an *optional* anchor has to -keep working when the plugin owning that anchor is not loaded. - -It is, however, **loud**. The relocation emits a -`nav_contribution_group_missing` diagnostic at `warn` — naming the contributing -package, the target app, the missing group id and the relocated items — so it -survives `OS_REGISTRY_LOG=warn` and appears in boot output. The same finding is -carried on the app itself, readable as -`registry.getAppNavDiagnostics(appName)`, and it is raised once per distinct -mis-aim rather than once per read of the app. - -`os build` answers the same question at compile time whenever the contributing -package and the target app are composed into one artifact, printing the finding -and carrying it in `--json` under `navigationGroupDiagnostics` (an empty array -when every contribution resolved). It **reports** there; it does not fail the -build. - -⚠️ The anchor id is the whole contract between a shell and its contributors, -and a contributor cannot see the shell's ids at authoring time. A typo -therefore produces a menu that renders, passes a smoke test, and has silently -moved the entry one level up — which is why the diagnostic exists rather than a -refusal. - The app itself ships from the dedicated `@objectstack/setup` package (package id `com.objectstack.setup`), which registers it at runtime. Per ADR-0048 (one app per package), `/apps/setup` — resolvable as @@ -108,6 +79,35 @@ A few notable entries: live in `plugin-audit`, but they are not contributed as Setup nav entries.) +### When a contribution names an anchor that is not there + +Aiming at a `group` id the target app does not declare is **not** refused and +the entry is **not** dropped: the items are appended at the app's **top level** +and the merge continues. That is deliberate — the merge is a read-time fold +precisely so registration order does not matter (a contributor may register +before the app it aims at), and a contribution into an *optional* anchor has to +keep working when the plugin owning that anchor is not loaded. + +It is, however, **loud**. The relocation emits a +`nav_contribution_group_missing` diagnostic at `warn` — naming the contributing +package, the target app, the missing group id and the relocated items — so it +survives `OS_REGISTRY_LOG=warn` and appears in boot output. The same finding is +carried on the app itself, readable as +`registry.getAppNavDiagnostics(appName)`, and it is raised once per distinct +mis-aim rather than once per read of the app. + +`os build` answers the same question at compile time whenever the contributing +package and the target app are composed into one artifact, printing the finding +and carrying it in `--json` under `navigationGroupDiagnostics` (an empty array +when every contribution resolved). It **reports** there; it does not fail the +build. + +⚠️ The anchor id is the whole contract between a shell and its contributors, +and a contributor cannot see the shell's ids at authoring time. A typo +therefore produces a menu that renders, passes a smoke test, and has silently +moved the entry one level up — which is why the diagnostic exists rather than a +refusal. + ## Why a shell + contributions The Setup App is a shell of empty group anchors rather than a fixed From 799d57e15872d74c1146724f554d62c48f0cbf53 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 3 Sep 2026 11:39:05 +0000 Subject: [PATCH 7/8] fix(cli): fill the declared `warnings` key instead of adding a payload key MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two standing pins caught the first cut and both are right: `build-json-advisory-parity.e2e.test.ts` and `build-json-undeclared-key-parity.e2e.test.ts`, each titled "adds NO new top-level key to the payload — this fills a declared key, it is not a new surface". #11643 and #11727 each faced this choice and filled `warnings`; that payload's shape is mirrored from `os validate --json` so a consumer reads one shape per class from either command. `navigationGroupDiagnostics` is gone and the findings ride `warnings`. A third pin in the same file settles what that implies: the only permitted residue between the two payloads is the structural advisory set, and "nothing rides in build that validate does not also report". So `os validate` computes the same list — which is the better answer on its own terms, since an author running `validate` should see a mis-aimed contribution exactly as one running `build` does. `findNavGroupDiagnostics` now derives the artifact's package entries itself (`artifactPackagesOf`), so both commands reach the check through one call that needs only the parsed stack, and the test uses that shipped derivation rather than a second copy of the id rule. `...navGroupWarnings` is APPENDED after `structuralWarnings` in validate, and that position is load-bearing: #12047's `the order lives at ONE site` pin matches the five existing members as contiguous source text, which is how it proves the order is defined once rather than re-spelled per exit. Appending keeps that pin guarding exactly what it was written to guard — no gate was loosened to get green. Reproduced first: 2 failed / 12 passed on the two key-set pins, naming `navigationGroupDiagnostics`; 14 passed after. The #12047 pin was then caught locally by the same discipline and is green with the other three (27 passed across the four files). Full cli unit project: 165 files / 2176 tests. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01UHvF5hyiZjnCyExFnfQB8m --- packages/cli/src/commands/compile.ts | 39 +++++++--------- packages/cli/src/commands/validate.ts | 39 ++++++++++++++++ .../src/utils/nav-contribution-groups.test.ts | 34 +++++++++----- .../cli/src/utils/nav-contribution-groups.ts | 44 ++++++++++++++++++- 4 files changed, 121 insertions(+), 35 deletions(-) diff --git a/packages/cli/src/commands/compile.ts b/packages/cli/src/commands/compile.ts index 86304554c8..fcbacb9f65 100644 --- a/packages/cli/src/commands/compile.ts +++ b/packages/cli/src/commands/compile.ts @@ -181,17 +181,23 @@ export default class Compile extends Command { let capProviderWarnings: Array<{ token: string; message: string }> = []; let unknownKeyWarnings: string[] = []; let docWarnings: DocIssue[] = []; - // [#14553] Build-only, so a SEPARATE payload key rather than a member of - // `warningsSoFar()` — the `bodyExtractionWarnings` precedent one field - // over, and for its stated reason: `os validate` never computes these, and - // folding a shape only ONE command can ship into the cross-command - // `warnings` key teaches consumers a shape the other command never emits. - let navGroupDiagnostics: NavContributionGroupDiagnostic[] = []; + // [#14553] A member of `warningsSoFar()`, NOT a payload key of its own. + // + // ⛔ The first cut made it a separate top-level key and two standing pins + // refused it by name — `build-json-advisory-parity` and + // `build-json-undeclared-key-parity`, both titled "adds NO new top-level + // key to the payload — this fills a declared key, it is not a new + // surface". #11643 and #11727 each faced this choice and filled + // `warnings`. `os validate` computes the same list, so the parity the + // third pin in that file asserts ("nothing rides in build that validate + // does not also report") holds rather than being weakened to fit. + let navGroupWarnings: NavContributionGroupDiagnostic[] = []; const warningsSoFar = () => [ ...ruleAdvisories, ...docWarnings, ...unknownKeyWarnings, ...capProviderWarnings, + ...navGroupWarnings, ]; // [#12125] The ADR-0087 D2 conversion notices, hoisted for the SAME reason // and under the SAME ruling as the four lists above — one field over. The @@ -474,18 +480,15 @@ export default class Compile extends Command { // yields nothing: contributing into an app another artifact ships is // the supported cross-artifact case, and is precisely why the merge // is a read-time fold. Only the composed case can be judged here. - navGroupDiagnostics = await findNavGroupDiagnostics( - result.data as Record, - packageEntries, - ); - if (navGroupDiagnostics.length > 0 && !flags.json) { + navGroupWarnings = await findNavGroupDiagnostics(result.data as Record); + if (navGroupWarnings.length > 0 && !flags.json) { console.log(''); printWarning( `Navigation contributions aimed at a group the target app does not declare ` + - `(${navGroupDiagnostics.length}) — the items still install, RELOCATED to the app's top level`, + `(${navGroupWarnings.length}) — the items still install, RELOCATED to the app's top level`, ); printBulletList( - navGroupDiagnostics.map((d) => `[${d.code}] ${d.message} Fix: ${d.fix}`), + navGroupWarnings.map((d) => `[${d.code}] ${d.message} Fix: ${d.fix}`), { noun: 'navigation-contribution diagnostic' }, ); } @@ -839,16 +842,6 @@ export default class Compile extends Command { // callable lowered cleanly, so a CI consumer can read the key // unconditionally. bodyExtractionWarnings: lowering.bodyExtractionWarnings, - // [#14553] Navigation contributions relocated past a missing group. - // A SEPARATE key for the same reason as the one above: `os validate` - // computes nothing of the kind, so folding these records into the - // cross-command `warnings` list would teach a consumer a shape only - // this command can ever ship. Empty array when every contribution - // resolved, so CI can read the key unconditionally — the records are - // the ADR-0038 BuildIssue-family entries the runtime fold raises - // (ADR-0112 D6c: a diagnostics code, lowercase and out of the error - // ledger), so one consumer reads one shape from either door. - navigationGroupDiagnostics: navGroupDiagnostics, // Same key `os validate --json` uses, so a CI consumer reads one shape // from either command rather than learning two. conversions: conversionNotices, diff --git a/packages/cli/src/commands/validate.ts b/packages/cli/src/commands/validate.ts index 9bc527a104..6d9ae54023 100644 --- a/packages/cli/src/commands/validate.ts +++ b/packages/cli/src/commands/validate.ts @@ -30,11 +30,17 @@ import { formatZodErrors, collectMetadataStats, printMetadataStats, + printWarning, + printBulletList, emitJson, isExitSignal, errorCodeFields, } from '../utils/format.js'; import { checkProtocolVersionGap } from '../utils/protocol-version-gap.js'; +// [#14553] The navigation-contribution group check, shared with `os compile`. +// Reports; never refuses — the runtime still relocates, deliberately. +import { findNavGroupDiagnostics } from '../utils/nav-contribution-groups.js'; +import type { NavContributionGroupDiagnostic } from '@objectstack/objectql'; export default class Validate extends Command { static override description = @@ -120,12 +126,25 @@ export default class Validate extends Command { let unknownKeyWarnings: string[] = []; let docWarnings: DocIssue[] = []; let structuralWarnings: string[] = []; + // [#14553] Computed HERE as well as in `os compile`, not only there. The + // #11727 residue pin asserts that nothing rides in build's `warnings` that + // validate does not also report, and the two commands being one wall with + // two doors is the #4409 / #4463 discipline this list already follows. + let navGroupWarnings: NavContributionGroupDiagnostic[] = []; const warningsSoFar = () => [ ...ruleAdvisories, ...docWarnings, ...unknownKeyWarnings, ...capProviderWarnings, ...structuralWarnings, + // [#14553] APPENDED, and the position is load-bearing. #12047's + // `the order lives at ONE site` pin matches the five members above as + // CONTIGUOUS source text — that is how it proves the order is defined + // once rather than re-spelled per exit. Slotting a sixth member (or even + // a comment) between them breaks that match, so a new member goes on the + // end and the pin keeps guarding exactly what it was written to guard. + // ⛔ Do not "fix" that pin by loosening its regex. + ...navGroupWarnings, ]; // [#12125] The ADR-0087 D2 conversion notices, hoisted for the SAME reason // and under the SAME ruling as the five lists above — one field over. The @@ -269,6 +288,26 @@ export default class Validate extends Command { // an advisory `pnpm add` hint. Mirrors the `os build` gate exactly. // // Not a registry rule: it reads `node_modules`, not the stack. + // [#14553] Navigation contributions whose `group` names no group in an + // app this same compilation unit ships. Reports, never refuses: the + // runtime relocates the items to the app's top level deliberately + // (the read-time fold stays order-independent, contributions into + // optional groups keep working), so what was missing was visibility, + // not a gate. A contribution aimed at an app no package here ships is + // NOT reported — that is the supported cross-artifact case. + navGroupWarnings = await findNavGroupDiagnostics(result.data as Record); + if (navGroupWarnings.length > 0 && !flags.json) { + console.log(''); + printWarning( + `Navigation contributions aimed at a group the target app does not declare ` + + `(${navGroupWarnings.length}) — the items still install, RELOCATED to the app's top level`, + ); + printBulletList( + navGroupWarnings.map((d) => `[${d.code}] ${d.message} Fix: ${d.fix}`), + { noun: 'navigation-contribution diagnostic' }, + ); + } + if (!flags.json) printStep('Checking capability providers (#3366)...'); const capProviderPreflight = preflightRequiredCapabilities({ requires: Array.isArray((config as { requires?: unknown[] }).requires) diff --git a/packages/cli/src/utils/nav-contribution-groups.test.ts b/packages/cli/src/utils/nav-contribution-groups.test.ts index 25f7edfa7d..043c0f27b5 100644 --- a/packages/cli/src/utils/nav-contribution-groups.test.ts +++ b/packages/cli/src/utils/nav-contribution-groups.test.ts @@ -54,7 +54,7 @@ import '@objectstack/objectql/core'; import { describe, it, expect } from 'vitest'; import { composeStacks, normalizeStackInput, ObjectStackDefinitionSchema } from '@objectstack/spec'; -import { collectNavGroupInputs, findNavGroupDiagnostics } from './nav-contribution-groups.js'; +import { artifactPackagesOf, collectNavGroupInputs, findNavGroupDiagnostics } from './nav-contribution-groups.js'; type AnyRec = Record; @@ -141,15 +141,15 @@ const parsedArtifact = (group: string): AnyRec => { return result.data as unknown as AnyRec; }; -/** The artifact's packages, in the `{ index, id, body }` shape `compile.ts` walks. */ -const packagesOf = (parsed: AnyRec) => - ((parsed.packages ?? []) as Array<{ manifest?: AnyRec }>).map((entry, index) => { - const body = (entry.manifest ?? {}) as AnyRec; - const id = typeof body.id === 'string' && body.id !== '' - ? body.id - : (typeof body.name === 'string' ? body.name : `packages[${index}]`); - return { id, body }; - }); +/** + * The artifact's packages, through the SHIPPED derivation. + * + * ⛔ Not a local re-spelling of the id rule. A second copy here would let this + * 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. + */ +const packagesOf = (parsed: AnyRec) => artifactPackagesOf(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 () => { @@ -172,6 +172,20 @@ describe('#14553 — `os build` checks `navigationContributions[].group` across expect(await findNavGroupDiagnostics(artifact(GROUP), packagesOf(artifact(GROUP)))).toEqual([]); }); + 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 + // 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'); + expect(await findNavGroupDiagnostics(typod)).toEqual( + await findNavGroupDiagnostics(typod, packagesOf(typod)), + ); + expect(await findNavGroupDiagnostics(typod)).toHaveLength(1); + expect(await findNavGroupDiagnostics(artifact(GROUP))).toEqual([]); + }); + it('ONE typo\'d group id prints one diagnostic naming the package, the app, the group and the items', async () => { const parsed = artifact('sales_grp'); const found = await findNavGroupDiagnostics(parsed, packagesOf(parsed)); diff --git a/packages/cli/src/utils/nav-contribution-groups.ts b/packages/cli/src/utils/nav-contribution-groups.ts index c7f80f8a74..d4b626b38c 100644 --- a/packages/cli/src/utils/nav-contribution-groups.ts +++ b/packages/cli/src/utils/nav-contribution-groups.ts @@ -38,6 +38,27 @@ * up. Loaded LAZILY, and only for a stack that actually declares a * contribution: `os build`'s cold path should not pull the data engine in to * judge two empty arrays. + * + * ## Why the findings ride the declared `warnings` key, and why BOTH commands + * ## compute them + * + * The first cut gave `os build --json` a new top-level + * `navigationGroupDiagnostics` key. Two standing pins caught it — + * `build-json-advisory-parity.e2e.test.ts` and + * `build-json-undeclared-key-parity.e2e.test.ts`, both titled "adds NO new + * top-level key to the payload — this fills a declared key, it is not a new + * surface". #11643 and #11727 each faced the same choice and filled + * `warnings`; that payload's shape is MIRRORED from `os validate --json` so a + * consumer reads one shape per class from either command rather than learning + * two. + * + * A third pin in the same file settles the other half: the ONLY permitted + * residue between the two payloads is the structural advisory set, and + * "nothing rides in build that validate does not also report". Folding these + * into build's `warnings` alone would therefore have turned THAT pin red. So + * both commands compute them — which is the better answer on its own terms + * too: an author running `os validate` sees the mis-aim exactly as one running + * `os build` does. */ import type { NavContributionGroupDiagnostic } from '@objectstack/objectql'; @@ -48,12 +69,31 @@ 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 as `compile.ts` walks them. */ +/** 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. + * + * 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. + */ +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 }; + }); +} + /** * The apps and the contributions visible in ONE compilation unit. * @@ -130,7 +170,7 @@ export function collectNavGroupInputs( */ export async function findNavGroupDiagnostics( parsed: AnyRec, - packages: readonly CompiledPackage[], + packages: readonly CompiledPackage[] = artifactPackagesOf(parsed), ): Promise { const { apps, contributions } = collectNavGroupInputs(parsed, packages); if (contributions.length === 0 || apps.length === 0) return []; From d22e7541d4664ead7d1f089101c294f1926d05b4 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 3 Sep 2026 12:10:32 +0000 Subject: [PATCH 8/8] docs: correct four places whose prose the payload-shape fix falsified MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit All four were self-inflicted by the previous commit, which deleted the `navigationGroupDiagnostics` key and made `os validate` compute the list — and all four are exactly the class the docs-drift bot says it cannot detect, since a page stating a rule by its inputs shares no identifier with the emitter. 1. `content/docs/ui/setup-app.mdx` documented the deleted key. That page is what a contributor reads before authoring an anchor id, so it is the worst place in the repo for the sentence to be wrong. It now says the findings ride the declared `warnings` key and that `os validate` reports them too. 2. The changeset named the deleted key. This one propagates: the changeset is this PR's input to the release notes, so a wrong key here would have become a wrong key in a published release. 3. `nav-contribution-diagnostics.ts` claimed this file is registered in `check-error-code-casing`'s `EXEMPT_FILES`. It is not, and the reason is the discovery the previous commits recorded: the code is REFERENCED at the stamp (`objlitconst`), so that gate's lowercase delegation never reaches this position and `dispatcher-error-vocabulary` classifies it instead. The docstring was the pre-discovery version and sent the next reader to the wrong file to look for a row that is not in it. It now describes what was actually done, and says not to add an `EXEMPT_FILES` entry — that list exempts files from a gate this one does not trip. 4. The vocabulary row's own `why` prose carried the dead key. Corrected to the `warnings` list of both commands; the four-test D6c argument is untouched. The historical note in `nav-contribution-groups.ts` keeps the old key name on purpose — it describes what the first cut did and why the pins rejected it. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01UHvF5hyiZjnCyExFnfQB8m --- ...ontribution-group-relocation-diagnostic.md | 13 ++++----- content/docs/ui/setup-app.mdx | 12 +++++---- .../src/nav-contribution-diagnostics.ts | 27 ++++++++++++++----- .../src/dispatcher-error-vocabulary.ts | 7 ++--- 4 files changed, 39 insertions(+), 20 deletions(-) diff --git a/.changeset/nav-contribution-group-relocation-diagnostic.md b/.changeset/nav-contribution-group-relocation-diagnostic.md index 18d68d0351..4a7f199ee6 100644 --- a/.changeset/nav-contribution-group-relocation-diagnostic.md +++ b/.changeset/nav-contribution-group-relocation-diagnostic.md @@ -33,12 +33,13 @@ app, the missing group id and the relocated items: the fold runs on every read of the app, and a line printed per request is as unreadable as one never printed. A deployment that asks for `silent` still gets silence, and still keeps the record. -- **At build time.** `os build` answers the same question over a composed - artifact, through the same predicate, and reports the same finding where an - author sees it first — in the text output and in `--json` under - `navigationGroupDiagnostics`. A contribution aimed at an app no package in - the artifact ships is not reported: contributing into an app another artifact - installs is the supported case, and is why the merge is a fold. +- **At authoring time.** `os build` and `os validate` answer the same question + over a composed artifact, through the same predicate, and report the same + finding where an author sees it first — in the text output and in `--json` + under the existing `warnings` key, beside the authoring-rule advisories and + capability hints. A contribution aimed at an app no package in the artifact + ships is not reported: contributing into an app another artifact installs is + the supported case, and is why the merge is a fold. **Nothing is refused.** No new failure, no ordering constraint, no change to what installs or to what `os build` accepts — a diagnostic was added and a diff --git a/content/docs/ui/setup-app.mdx b/content/docs/ui/setup-app.mdx index 1f85efcea8..40d0a28637 100644 --- a/content/docs/ui/setup-app.mdx +++ b/content/docs/ui/setup-app.mdx @@ -96,11 +96,13 @@ carried on the app itself, readable as `registry.getAppNavDiagnostics(appName)`, and it is raised once per distinct mis-aim rather than once per read of the app. -`os build` answers the same question at compile time whenever the contributing -package and the target app are composed into one artifact, printing the finding -and carrying it in `--json` under `navigationGroupDiagnostics` (an empty array -when every contribution resolved). It **reports** there; it does not fail the -build. +`os build` **and `os validate`** answer the same question at compile time +whenever the contributing package and the target app are composed into one +artifact. Both print the finding and both carry it in `--json` under the +existing `warnings` key, beside the authoring-rule advisories and the +capability hints — the payload is deliberately closed, so a new class of +finding fills a declared key rather than adding one. They **report** there; +neither fails the build. ⚠️ The anchor id is the whole contract between a shell and its contributors, and a contributor cannot see the shell's ids at authoring time. A typo diff --git a/packages/objectql/src/nav-contribution-diagnostics.ts b/packages/objectql/src/nav-contribution-diagnostics.ts index 00ac19ae59..45477a98c8 100644 --- a/packages/objectql/src/nav-contribution-diagnostics.ts +++ b/packages/objectql/src/nav-contribution-diagnostics.ts @@ -55,12 +55,27 @@ * `metadata-protocol/src/build-probes.ts`), which is also what lets one stream * carry the runtime and the build finding. * - * ⛔ So: no `ERROR_CODE_LEDGER` registration and no `UNREGISTERED_CODE_SITES` - * row. `check:dispatcher-error-vocabulary` delegates a lowercase literal in an - * object-literal `code:` position to `check:error-code-casing`, which owns the - * D6/D6b/D6c discrimination through its `EXEMPT_FILES` list — this file is - * registered there with that reason, the same way `build-probes.ts` and - * `metadata-diagnostics.ts` are. + * ⛔ So: **no `ERROR_CODE_LEDGER` registration.** What it does need is a + * CLASSIFICATION row, and which gate wants one was measured rather than + * assumed — the first guess was wrong in a way worth recording. + * + * `check:dispatcher-error-vocabulary` delegates a lowercase literal in an + * object-literal `code:` position to `check:error-code-casing`, whose + * `EXEMPT_FILES` list carries the D6/D6b/D6c discrimination for + * `build-probes.ts` and `metadata-diagnostics.ts`. That delegation does NOT + * reach this file: the code is exported as a named constant and REFERENCED at + * the stamp (`code: NAV_CONTRIBUTION_GROUP_MISSING`), which is the + * `objlitconst` shape, and every lowercase pattern that gate delegates needs a + * quoted literal beside the token. So `check:error-code-casing` passes this + * file with no entry — it never sees the value — and + * `check:dispatcher-error-vocabulary` is the gate that reports it. + * + * ⇒ The row lives in `packages/runtime/src/dispatcher-error-vocabulary.ts`, + * verdict `foreign-vocabulary`, door `none`, carrying the four-test D6c + * argument above. ⛔ Do not add this file to `EXEMPT_FILES`: that list exempts + * files from a gate this one does not trip, so an entry there would assert a + * discrimination nothing is making and hide the real row's absence if it were + * ever deleted. */ /** diff --git a/packages/runtime/src/dispatcher-error-vocabulary.ts b/packages/runtime/src/dispatcher-error-vocabulary.ts index fc98977ab9..049924963d 100644 --- a/packages/runtime/src/dispatcher-error-vocabulary.ts +++ b/packages/runtime/src/dispatcher-error-vocabulary.ts @@ -318,9 +318,10 @@ export const UNREGISTERED_CODE_SITES: readonly UnregisteredCodeSite[] = [ verdict: 'foreign-vocabulary', why: 'ADR-0112 D6c by name, on all four of its tests: it ships as PAYLOAD of a success (carried on ' + - 'the served app, and on `os build --json` under `navigationGroupDiagnostics`, from runs that ' + - 'exit 0), it describes an ARTIFACT rather than a request (which app, which package, which ' + - 'group id), its severity is `warning` and cannot be anything else, and no path routes it to ' + + 'the served app, and in the `warnings` list of `os build --json` AND `os validate --json`, ' + + 'from runs that exit 0), it describes an ARTIFACT rather than a request (which app, which ' + + 'package, which group id), its severity is `warning` and cannot be anything else, and no ' + + 'path routes it to ' + '`error.code` — the producer is a read-time FOLD (`applyNavContributions`) and a compile ' + 'step, neither of which throws. The card that added it added the diagnostic and ' + 'deliberately added NO refusal: the contribution is still relocated to the app\'s top ' +