fix(objectql,cli): a navigation contribution relocated past a missing group now says so, at warn and at build time

.changeset/nav-contribution-group-relocation-diagnostic.md

@@ -0,0 +1,51 @@
+---
+"@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 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
+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.

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`.

content/docs/ui/setup-app.mdx

@@ -79,6 +79,37 @@ 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` **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
+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

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' },
+          ],
+        },
       ],
     },
   ],

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: [

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,11 +181,23 @@ export default class Compile extends Command {
     let capProviderWarnings: Array<{ token: string; message: string }> = [];
     let unknownKeyWarnings: string[] = [];
     let docWarnings: DocIssue[] = [];
+    // [#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
@@ -446,6 +462,37 @@ 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.
+      navGroupWarnings = await findNavGroupDiagnostics(result.data as Record<string, unknown>);
+      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' },
+        );
+      }
+
       // 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

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<string, unknown>);
+      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)