Merged
51 changes: 51 additions & 0 deletions .changeset/nav-contribution-group-relocation-diagnostic.md
Original file line numberDiff line numberDiff line change
@@ -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.
2 changes: 1 addition & 1 deletion content/docs/permissions/system-context.mdx
Original file line numberDiff line numberDiff line change
Expand Up@@ -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`.
Expand Down
31 changes: 31 additions & 0 deletions content/docs/ui/setup-app.mdx
Original file line numberDiff line numberDiff line change
Expand Up@@ -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
Expand Down
14 changes: 13 additions & 1 deletion examples/app-multi-package/src/packages/core/index.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -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' },
],
},
],
},
],
Expand Down
29 changes: 29 additions & 0 deletions examples/app-multi-package/src/packages/orders/index.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -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: {
Expand All@@ -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: [
Expand Down
47 changes: 47 additions & 0 deletions packages/cli/src/commands/compile.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -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).
Expand DownExpand Up@@ -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
Expand DownExpand Up@@ -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
Expand Down
39 changes: 39 additions & 0 deletions packages/cli/src/commands/validate.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -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 =
Expand DownExpand Up@@ -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
Expand DownExpand Up@@ -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)
Expand Down
Loading
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Skip to content
Merged
51 changes: 51 additions & 0 deletions .changeset/nav-contribution-group-relocation-diagnostic.md
Original file line numberDiff line numberDiff line change
@@ -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.
2 changes: 1 addition & 1 deletion content/docs/permissions/system-context.mdx
Original file line numberDiff line numberDiff line change
Expand Up@@ -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`.
Expand Down
31 changes: 31 additions & 0 deletions content/docs/ui/setup-app.mdx
Original file line numberDiff line numberDiff line change
Expand Up@@ -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
Expand Down
14 changes: 13 additions & 1 deletion examples/app-multi-package/src/packages/core/index.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -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' },
],
},
],
},
],
Expand Down
29 changes: 29 additions & 0 deletions examples/app-multi-package/src/packages/orders/index.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -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: {
Expand All@@ -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: [
Expand Down
47 changes: 47 additions & 0 deletions packages/cli/src/commands/compile.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -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).
Expand DownExpand Up@@ -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
Expand DownExpand Up@@ -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
Expand Down
39 changes: 39 additions & 0 deletions packages/cli/src/commands/validate.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -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 =
Expand DownExpand Up@@ -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
Expand DownExpand Up@@ -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)
Expand Down
Loading
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix"); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content
Merged
51 changes: 51 additions & 0 deletions .changeset/nav-contribution-group-relocation-diagnostic.md
Original file line numberDiff line numberDiff line change
@@ -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.
2 changes: 1 addition & 1 deletion content/docs/permissions/system-context.mdx
Original file line numberDiff line numberDiff line change
Expand Up@@ -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`.
Expand Down
31 changes: 31 additions & 0 deletions content/docs/ui/setup-app.mdx
Original file line numberDiff line numberDiff line change
Expand Up@@ -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
Expand Down
14 changes: 13 additions & 1 deletion examples/app-multi-package/src/packages/core/index.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -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' },
],
},
],
},
],
Expand Down
29 changes: 29 additions & 0 deletions examples/app-multi-package/src/packages/orders/index.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -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: {
Expand All@@ -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: [
Expand Down
47 changes: 47 additions & 0 deletions packages/cli/src/commands/compile.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -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).
Expand DownExpand Up@@ -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
Expand DownExpand Up@@ -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
Expand Down
39 changes: 39 additions & 0 deletions packages/cli/src/commands/validate.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -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 =
Expand DownExpand Up@@ -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
Expand DownExpand Up@@ -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)
Expand Down
Loading
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms"); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content
Merged
51 changes: 51 additions & 0 deletions .changeset/nav-contribution-group-relocation-diagnostic.md
Original file line numberDiff line numberDiff line change
@@ -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.
2 changes: 1 addition & 1 deletion content/docs/permissions/system-context.mdx
Original file line numberDiff line numberDiff line change
Expand Up@@ -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`.
Expand Down
31 changes: 31 additions & 0 deletions content/docs/ui/setup-app.mdx
Original file line numberDiff line numberDiff line change
Expand Up@@ -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
Expand Down
14 changes: 13 additions & 1 deletion examples/app-multi-package/src/packages/core/index.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -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' },
],
},
],
},
],
Expand Down
29 changes: 29 additions & 0 deletions examples/app-multi-package/src/packages/orders/index.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -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: {
Expand All@@ -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: [
Expand Down
47 changes: 47 additions & 0 deletions packages/cli/src/commands/compile.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -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).
Expand DownExpand Up@@ -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
Expand DownExpand Up@@ -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
Expand Down
39 changes: 39 additions & 0 deletions packages/cli/src/commands/validate.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -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 =
Expand DownExpand Up@@ -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
Expand DownExpand Up@@ -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)
Expand Down
Loading
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links"); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
Skip to content
Merged
51 changes: 51 additions & 0 deletions .changeset/nav-contribution-group-relocation-diagnostic.md
Original file line numberDiff line numberDiff line change
@@ -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.
2 changes: 1 addition & 1 deletion content/docs/permissions/system-context.mdx
Original file line numberDiff line numberDiff line change
Expand Up@@ -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`.
Expand Down
31 changes: 31 additions & 0 deletions content/docs/ui/setup-app.mdx
Original file line numberDiff line numberDiff line change
Expand Up@@ -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
Expand Down
14 changes: 13 additions & 1 deletion examples/app-multi-package/src/packages/core/index.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -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' },
],
},
],
},
],
Expand Down
29 changes: 29 additions & 0 deletions examples/app-multi-package/src/packages/orders/index.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -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: {
Expand All@@ -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: [
Expand Down
47 changes: 47 additions & 0 deletions packages/cli/src/commands/compile.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -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).
Expand DownExpand Up@@ -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
Expand DownExpand Up@@ -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
Expand Down
39 changes: 39 additions & 0 deletions packages/cli/src/commands/validate.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -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 =
Expand DownExpand Up@@ -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
Expand DownExpand Up@@ -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)
Expand Down
Loading
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default"); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content
Merged
51 changes: 51 additions & 0 deletions .changeset/nav-contribution-group-relocation-diagnostic.md
Original file line numberDiff line numberDiff line change
@@ -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.
2 changes: 1 addition & 1 deletion content/docs/permissions/system-context.mdx
Original file line numberDiff line numberDiff line change
Expand Up@@ -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`.
Expand Down
31 changes: 31 additions & 0 deletions content/docs/ui/setup-app.mdx
Original file line numberDiff line numberDiff line change
Expand Up@@ -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
Expand Down
14 changes: 13 additions & 1 deletion examples/app-multi-package/src/packages/core/index.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -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' },
],
},
],
},
],
Expand Down
29 changes: 29 additions & 0 deletions examples/app-multi-package/src/packages/orders/index.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -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: {
Expand All@@ -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: [
Expand Down
47 changes: 47 additions & 0 deletions packages/cli/src/commands/compile.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -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).
Expand DownExpand Up@@ -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
Expand DownExpand Up@@ -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
Expand Down
39 changes: 39 additions & 0 deletions packages/cli/src/commands/validate.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -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 =
Expand DownExpand Up@@ -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
Expand DownExpand Up@@ -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)
Expand Down
Loading
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content
Merged
51 changes: 51 additions & 0 deletions .changeset/nav-contribution-group-relocation-diagnostic.md
Original file line numberDiff line numberDiff line change
@@ -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.
2 changes: 1 addition & 1 deletion content/docs/permissions/system-context.mdx
Original file line numberDiff line numberDiff line change
Expand Up@@ -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`.
Expand Down
31 changes: 31 additions & 0 deletions content/docs/ui/setup-app.mdx
Original file line numberDiff line numberDiff line change
Expand Up@@ -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
Expand Down
14 changes: 13 additions & 1 deletion examples/app-multi-package/src/packages/core/index.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -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' },
],
},
],
},
],
Expand Down
29 changes: 29 additions & 0 deletions examples/app-multi-package/src/packages/orders/index.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -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: {
Expand All@@ -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: [
Expand Down
47 changes: 47 additions & 0 deletions packages/cli/src/commands/compile.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -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).
Expand DownExpand Up@@ -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
Expand DownExpand Up@@ -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
Expand Down
39 changes: 39 additions & 0 deletions packages/cli/src/commands/validate.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -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 =
Expand DownExpand Up@@ -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
Expand DownExpand Up@@ -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)
Expand Down
Loading
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
Skip to content
Merged
51 changes: 51 additions & 0 deletions .changeset/nav-contribution-group-relocation-diagnostic.md
Original file line numberDiff line numberDiff line change
@@ -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.
2 changes: 1 addition & 1 deletion content/docs/permissions/system-context.mdx
Original file line numberDiff line numberDiff line change
Expand Up@@ -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`.
Expand Down
31 changes: 31 additions & 0 deletions content/docs/ui/setup-app.mdx
Original file line numberDiff line numberDiff line change
Expand Up@@ -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
Expand Down
14 changes: 13 additions & 1 deletion examples/app-multi-package/src/packages/core/index.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -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' },
],
},
],
},
],
Expand Down
29 changes: 29 additions & 0 deletions examples/app-multi-package/src/packages/orders/index.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -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: {
Expand All@@ -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: [
Expand Down
47 changes: 47 additions & 0 deletions packages/cli/src/commands/compile.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -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).
Expand DownExpand Up@@ -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
Expand DownExpand Up@@ -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
Expand Down
39 changes: 39 additions & 0 deletions packages/cli/src/commands/validate.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -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 =
Expand DownExpand Up@@ -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
Expand DownExpand Up@@ -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)
Expand Down
Loading
Loading