diff --git a/.changeset/app-navigation-strict.md b/.changeset/app-navigation-strict.md new file mode 100644 index 0000000000..85de1c2e47 --- /dev/null +++ b/.changeset/app-navigation-strict.md @@ -0,0 +1,48 @@ +--- +"@objectstack/spec": major +"@objectstack/platform-objects": patch +--- + +feat(spec)!: reject unknown keys across the app shell and navigation tree (#4001 app step, PR B) + +Closes the last high-traffic authorable surface in the unknown-key strictness +ratchet (flow + permission #4071, RLS / sharing / position #4099, approval +#4119, App dead-key tombstones #4142). The app shell is the densest +hand-authored surface on the platform — a navigation tree is where an author +or AI is most likely to write a key from memory — so a silent strip here was +the most probable instance of the #3405 trap. + +- **`AppSchema`** and its sub-schemas (`AppBrandingSchema`, + `NavigationAreaSchema`, `AppContextSelectorSchema` + its `optionsSource` / + `filter` blocks, `NavigationContributionSchema`) are `.strict()`. +- **`NavigationItemSchema` becomes a DISCRIMINATED union on `type`.** This is + what makes strict readable: a plain union of strict members answers one + unknown key with an `invalid_union` aggregate naming all nine branches, + while discriminating on `type` first yields a single `unrecognized_keys` + issue against the branch the author actually wrote — at an exact path + through nested `children` — and a mistyped `type` gets its own "Invalid + discriminator value". Each variant carries its own suggestion pool, so a + `url` item is never told about `dashboardName`. +- **Still OPEN by design:** `PageNavItem.params`, `ComponentNavItem.params` + and `ActionNavItem.actionDef.params` — per-target payloads owned by the + page / component / action, not by the nav item. + +**A real defect the gate caught, in the platform's own app:** `ACCOUNT_APP` +declared `defaultOpen` on three navigation groups. That was never a schema +key — `expanded` is — so all three shipped COLLAPSED while their author +believed they opened by default. Fixed at the producer (contract-first) and +`defaultOpen` / `open` / `collapsed` / `isOpen` now alias to `expanded`. + +**Migration.** Any key now rejected was previously stripped and had no +runtime effect. The error carries the fix; mappings include +`menu`/`sidebar`/`tabs`/`items` → `navigation`, `title` → `label`, +`permissions` → `requiredPermissions`, `sort`/`position` → `order`, +`defaultOpen` → `expanded`, `args` → `params` (actionDef), `primary` → +`primaryColor`, `url` → `endpoint` (options source), plus wrong-layer +pointers: `pages`/`views`/`flows` are not App fields, and a payload named on +the wrong variant points at the `type` that owns it. + +The `visibleWhen` → `visible` alias is the load-bearing one: ADR-0089 made +`visibleWhen` canonical on view/page schemas, so an author who learned it +there would silently lose a nav entry's visibility gate — a capability gate +failing open, the worst shape of the silent-strip bug. diff --git a/content/docs/releases/v17.mdx b/content/docs/releases/v17.mdx index e3abaca579..6a205a203b 100644 --- a/content/docs/releases/v17.mdx +++ b/content/docs/releases/v17.mdx @@ -307,7 +307,7 @@ model), while `url` platform-wide means an HTTP endpoint to call (`http` node, webhooks). The singular `input` on `map` / `subflow` / `connector_action` is those nodes' own canonical key and is untouched. -### Flow, permission, RLS, sharing, position and approval schemas reject unknown keys (#4001) +### Authorable schemas reject unknown keys (#4001) Zod's default is `.strip`: a key a schema does not declare is silently discarded and the instance keeps parsing. On an authorable surface that is the @@ -341,6 +341,15 @@ schema to the two highest-risk authorable surfaces, per the triage in a deliberately flat position (ADR-0090 D3). Position also gains the `protection` block and ADR-0010 runtime envelope every sibling registered type already declared. +- **The app shell** — `AppSchema`, its branding / area / context-selector / + contribution blocks, and the whole navigation tree. The nav-item union is + now DISCRIMINATED on `type`, so one unknown key yields one precise issue + against the branch you wrote (at an exact path through nested `children`) + rather than a nine-branch `invalid_union` wall. Per-target payloads + (`params` on page / component / action items) stay open. The gate's first + catch here was in the platform's own Account app: three navigation groups + declared `defaultOpen` — never a schema key — and so shipped collapsed + while their author believed they opened by default (`expanded` is the key). - **Approval nodes** — all four authoring schemas (node config, approver, escalation, decision-output). Process-era keys carry the ADR-0019 re-home map (`steps` → successive approval nodes, `entryCriteria` → the entering diff --git a/content/docs/ui/apps.mdx b/content/docs/ui/apps.mdx index 002a74d597..8d694a86e9 100644 --- a/content/docs/ui/apps.mdx +++ b/content/docs/ui/apps.mdx @@ -57,7 +57,7 @@ const crmApp = { ## Navigation Items -The navigation tree supports eight item types, combined to create rich menu structures. The most common are shown below (`object`, `dashboard`, `page`, `url`, `group`); the spec also defines `report`, `action`, and `component` items. +The navigation tree supports nine item types, combined to create rich menu structures. The most common are shown below (`object`, `dashboard`, `page`, `url`, `group`, `separator`); the spec also defines `report`, `action`, and `component` items. ### Object Navigation @@ -121,9 +121,23 @@ Groups items into collapsible sections with children: } ``` +### Separator + +A visual divider in the navigation list. It renders no target and carries no +label — the only keys it accepts are `type`, an optional `id`, and an optional +`order`: + +```typescript +{ type: 'separator', order: 30 } +``` + ### Common Navigation Properties -All navigation items share these base properties. Every item **must** declare a unique `id` (lowercase `snake_case`) — it is required by the schema and is referenced by `homePageId` and `mobileNavigation.bottomNavItems`: +All navigation items **except `separator`** share these base properties. Every +item **must** declare a unique `id` (lowercase `snake_case`) — it is required by +the schema and is referenced by `homePageId` and `mobileNavigation.bottomNavItems`. +(On a `separator`, `id` is optional and the remaining properties below are +rejected — a divider has nothing to label, gate, or badge.) | Property | Type | Description | | :--- | :--- | :--- | diff --git a/docs/audits/2026-07-unknown-key-strictness-ledger.md b/docs/audits/2026-07-unknown-key-strictness-ledger.md index 692bcc8478..1bd1542226 100644 --- a/docs/audits/2026-07-unknown-key-strictness-ledger.md +++ b/docs/audits/2026-07-unknown-key-strictness-ledger.md @@ -92,6 +92,16 @@ dropped at parse, and nothing failed. faith, silently discarded, believed to be in effect. Removed (the identical stamp on the fixture's *object* is real and stays). +5. **`position.test.ts` asserted a fictional hierarchy** (step 2) — see the + entry below. +6. **The platform's own Account app declared `defaultOpen` on three navigation + groups** (app step, PR B). `expanded` is the schema key; `defaultOpen` + never was — so all three groups shipped COLLAPSED while their author + believed they opened by default. Fixed at the producer, and the spelling + now aliases to `expanded`. Note where this one was found: not in a tenant + project, but in first-party platform metadata that had been shipping for + releases. + This is the empirical argument for the ratchet: the inference "no metadata in the repo carries unknown keys" was **false three times over**, and only the strict gate could prove it. Note the asymmetry in the two schema gaps — both @@ -127,7 +137,7 @@ tightening (the #4001 "sharing-rule lesson": candidates, not verdicts). | `view.zod.ts` | 51 | authorable | partially strict (ADR-0089); long tail of sub-blocks | | `component.zod.ts` | 29 | authorable | **next candidate** — SDUI component defs; check React-prop open slots first (p) | | `theme.zod.ts` | 14 | authorable (p) | authored themes | -| `app.zod.ts` | 11 | authorable | **PR A done (#4001 app step): the seven audit-dead keys (`version`/`aria`/`objects`/`apis`/`sharing`/`embed`/`mobileNavigation`) are `retiredKey()` tombstones + an ADR-0087 conversion** — the ADR-0049 precondition for strict. **PR B next**: `AppSchema` + nav union `.strict()`; the union-error question is settled — convert `NavigationItemSchema` to `z.discriminatedUnion('type', …)` (verified: matched-branch-only unknown-key errors, precise recursive paths, `toJSONSchema` clean) | +| `app.zod.ts` | 11 | authorable | **strict as of #4001 PR B** — `AppSchema` + branding / area / context-selector / contribution, and the nav-item union converted to `z.discriminatedUnion('type', …)` (the union-error question, settled empirically: matched-branch-only errors, exact recursive paths, `toJSONSchema` clean). Per-target `params` stay open. PR A (#4142) tombstoned the seven audit-dead keys first | | `dashboard.zod.ts` | 11 | authorable | partially strict | | `widget.zod.ts` | 9 | authorable (p) | | | `page.zod.ts` | 7 | authorable | partially strict (ADR-0089) | @@ -203,9 +213,12 @@ tightening (the #4001 "sharing-rule lesson": candidates, not verdicts). ## Next steps (verify-then-enforce, one shape at a time) -1. `ui/app.zod.ts` — `AppSchema` + navigation union (highest-traffic remaining - authorable type; needs union-error design so the strict error is readable). -2. `data/hook.zod.ts`, `data/datasource.zod.ts` — `defineHook` / stack config. +1. `data/hook.zod.ts`, `data/datasource.zod.ts` — `defineHook` / stack config + (both still provisional (p) classifications — verify before tightening). +2. The `@objectstack/lint` unknown-key WARNING layer: non-breaking, shippable + in a minor, and it extends AI-detectable coverage to every remaining + authorable site at once while accumulating evidence (which keys real + tenant projects actually carry) for a v18 strict close-out. 3. Promote this ledger to a machine-checked gate (pattern of `packages/spec/liveness/` + `check:liveness`) once enough of the surface is classified that the table above is enforceable rather than descriptive. @@ -219,6 +232,10 @@ schemas, with the ADR-0019 re-home map as wrong-layer guidance (`steps` / `entryCriteria` / `onApprove` / `onReject` / `rejectionBehavior` each point at where the concept lives on the flow graph now). +Done in the app step, PR B: `AppSchema` + the navigation tree strict, via a +discriminated union — the union-error concern that deferred this step was +resolved by measurement, not design work. + Done in the app step, PR A: the seven audit-dead AppSchema keys tombstoned (`retiredKey` + `app-dead-authoring-keys-removed` conversion + step-17 migration entry), clearing the enforce-or-remove precondition for the app diff --git a/packages/platform-objects/src/apps/account.app.ts b/packages/platform-objects/src/apps/account.app.ts index 8325c347a9..b4f48adc79 100644 --- a/packages/platform-objects/src/apps/account.app.ts +++ b/packages/platform-objects/src/apps/account.app.ts @@ -80,7 +80,7 @@ export const ACCOUNT_APP: App = { type: 'group', label: 'Inbox', icon: 'inbox', - defaultOpen: true, + expanded: true, children: [ { // ADR-0030: the user-facing inbox is the materialization @@ -126,7 +126,7 @@ export const ACCOUNT_APP: App = { type: 'group', label: 'Security', icon: 'shield', - defaultOpen: true, + expanded: true, children: [ { id: 'nav_account_linked', @@ -154,7 +154,7 @@ export const ACCOUNT_APP: App = { type: 'group', label: 'Developer', icon: 'code', - defaultOpen: false, + expanded: false, children: [ { id: 'nav_account_api_keys', diff --git a/packages/spec/src/ui/app.test.ts b/packages/spec/src/ui/app.test.ts index 158d946230..0251865e84 100644 --- a/packages/spec/src/ui/app.test.ts +++ b/packages/spec/src/ui/app.test.ts @@ -2,6 +2,7 @@ import { describe, it, expect } from 'vitest'; import { AppSchema, AppBrandingSchema, + NavigationContributionSchema, NavigationItemSchema, NavigationAreaSchema, ObjectNavItemSchema, @@ -1056,3 +1057,188 @@ describe('retired dead keys carry prescriptions (#4001)', () => { })).not.toThrow(); }); }); + +// #4001 (app step, PR B) — the app shell is `.strict()`, and the nav-item +// union is DISCRIMINATED on `type` so strict stays readable: one unknown key +// yields one issue against the branch the author actually wrote, at an exact +// path, instead of an `invalid_union` wall naming all nine branches. +describe('unknown keys are rejected, not stripped (#4001 PR B)', () => { + const unknownKeyIssue = (schema: { safeParse: (v: unknown) => any }, value: unknown) => { + const result = schema.safeParse(value); + expect(result.success).toBe(false); + return result.error!.issues.find((i: { code: string }) => i.code === 'unrecognized_keys'); + }; + const navItem = (extra: Record) => ({ + name: 'app_x', label: 'X', + navigation: [{ id: 'nav_a', label: 'A', type: 'object', objectName: 'account', ...extra }], + }); + + describe('AppSchema', () => { + it('rejects an undeclared key instead of silently dropping it', () => { + expect(unknownKeyIssue(AppSchema, { name: 'app_a', label: 'A', notAKey: 1 })!.message) + .toContain('`notAKey`'); + }); + + it('points sidebar vocabulary at `navigation`', () => { + for (const key of ['menu', 'sidebar', 'tabs', 'items']) { + expect(unknownKeyIssue(AppSchema, { name: 'app_a', label: 'A', [key]: [] })!.message) + .toContain(`\`${key}\` → \`navigation\``); + } + }); + + it('explains where pages/views/flows really live', () => { + expect(unknownKeyIssue(AppSchema, { name: 'app_a', label: 'A', pages: [] })!.message) + .toContain("type: 'page'"); + expect(unknownKeyIssue(AppSchema, { name: 'app_a', label: 'A', views: [] })!.message) + .toContain('listViews'); + expect(unknownKeyIssue(AppSchema, { name: 'app_a', label: 'A', flows: [] })!.message) + .toContain('defineStack'); + }); + + it('keeps the PR A tombstone prescriptions rather than a bare strict error', () => { + const result = AppSchema.safeParse({ name: 'app_a', label: 'A', sharing: { enabled: true } }); + expect(result.success).toBe(false); + expect(result.error!.issues.map((i) => i.message).join('\n')).toContain('FormView.sharing'); + }); + + it('accepts every key the schema declares (guards APP_KEYS drift)', () => { + const probes: Record = { + description: 'd', icon: 'briefcase', branding: { primaryColor: '#fff' }, + active: false, isDefault: true, hidden: true, + navigation: [{ id: 'nav_a', label: 'A', type: 'object', objectName: 'account' }], + areas: [{ id: 'area_a', label: 'A', navigation: [] }], + contextSelectors: [{ id: 'pkg', label: 'Package', optionsSource: { endpoint: '/api/v1/packages' } }], + homePageId: 'nav_a', requiredPermissions: ['app.access.x'], + defaultAgent: 'ask', protection: { lock: 'none' }, + }; + for (const [key, value] of Object.entries(probes)) { + const result = AppSchema.safeParse({ name: 'app_a', label: 'A', [key]: value }); + const unknown = result.success + ? undefined + : result.error.issues.find((i) => i.code === 'unrecognized_keys'); + expect(unknown, `\`${key}\` should be a declared App key`).toBeUndefined(); + } + }); + }); + + describe('navigation items (discriminated union)', () => { + it('reports ONE issue against the matched branch, not a union wall', () => { + const result = AppSchema.safeParse(navItem({ viewname: 'all' })); + expect(result.success).toBe(false); + const unrecognized = result.error!.issues.filter((i) => i.code === 'unrecognized_keys'); + expect(unrecognized).toHaveLength(1); + expect(result.error!.issues.some((i) => i.code === 'invalid_union')).toBe(false); + expect(unrecognized[0].message).toContain('`viewname` → `viewName`'); + }); + + it('points `visibleWhen` at `visible` so a nav gate cannot fail open', () => { + // ADR-0089 made visibleWhen canonical on view/page; borrowing it here + // used to STRIP the gate and render the entry unconditionally. + expect(unknownKeyIssue(AppSchema, navItem({ visibleWhen: 'x == 1' }))!.message) + .toContain('`visibleWhen` → `visible`'); + }); + + it('points a wrong-variant payload at the type that owns it', () => { + const result = AppSchema.safeParse({ + name: 'app_a', label: 'A', + navigation: [{ id: 'nav_u', label: 'U', type: 'url', url: 'https://x', pageName: 'p' }], + }); + expect(result.success).toBe(false); + expect(result.error!.issues.find((i) => i.code === 'unrecognized_keys')!.message) + .toContain("type: 'page'"); + }); + + it('gives a precise path for an unknown key nested in children', () => { + const result = AppSchema.safeParse({ + name: 'app_a', label: 'A', + navigation: [{ + id: 'grp', label: 'G', type: 'group', + children: [{ id: 'nav_a', label: 'A', type: 'object', objectName: 'account', bogus: 1 }], + }], + }); + expect(result.success).toBe(false); + const issue = result.error!.issues.find((i) => i.code === 'unrecognized_keys'); + expect(issue!.path).toEqual(['navigation', 0, 'children', 0]); + }); + + it('reports a mistyped `type` as a discriminator error, not an unknown key', () => { + const result = AppSchema.safeParse({ + name: 'app_a', label: 'A', + navigation: [{ id: 'nav_a', label: 'A', type: 'objct', objectName: 'account' }], + }); + expect(result.success).toBe(false); + const message = result.error!.issues.map((i) => i.message).join('\n'); + expect(message).toContain('Invalid discriminator value'); + }); + + it('keeps the objectNavTargetExclusivity refinement working inside the union', () => { + const result = AppSchema.safeParse({ + name: 'app_a', label: 'A', + navigation: [{ + id: 'nav_a', label: 'A', type: 'object', objectName: 'account', + filters: { owner_id: '{current_user_id}' }, viewName: 'all', + }], + }); + expect(result.success).toBe(false); + expect(result.error!.issues.map((i) => i.message).join('\n')).toContain('pick ONE landing'); + }); + + it('leaves page/component/action params OPEN (the target owns that contract)', () => { + expect(() => AppSchema.parse({ + name: 'app_a', label: 'A', + navigation: [ + { id: 'nav_p', label: 'P', type: 'page', pageName: 'p', params: { anything: 1, nested: { x: 2 } } }, + { id: 'nav_c', label: 'C', type: 'component', componentRef: 'metadata:resource', params: { type: 'object' } }, + { id: 'nav_x', label: 'X', type: 'action', actionDef: { actionName: 'go', params: { free: 'form' } } }, + ], + })).not.toThrow(); + }); + + it('rejects unknown keys inside actionDef', () => { + const result = AppSchema.safeParse({ + name: 'app_a', label: 'A', + navigation: [{ id: 'nav_x', label: 'X', type: 'action', actionDef: { actionName: 'go', args: {} } }], + }); + expect(result.success).toBe(false); + expect(result.error!.issues.find((i) => i.code === 'unrecognized_keys')!.message) + .toContain('`args` → `params`'); + }); + + it('accepts every variant with its full declared payload', () => { + expect(() => AppSchema.parse({ + name: 'app_a', label: 'A', + navigation: [ + { id: 'n1', label: 'O', type: 'object', objectName: 'account', viewName: 'all', icon: 'x', order: 1, badge: '3', badgeVariant: 'default', visible: 'true', requiredPermissions: ['p'], requiresObject: 'account', requiresService: 'ai' }, + { id: 'n2', label: 'D', type: 'dashboard', dashboardName: 'd' }, + { id: 'n3', label: 'P', type: 'page', pageName: 'p' }, + { id: 'n4', label: 'U', type: 'url', url: 'https://x', target: '_blank' }, + { id: 'n5', label: 'R', type: 'report', reportName: 'r' }, + { id: 'n6', label: 'A', type: 'action', actionDef: { actionName: 'go' } }, + { id: 'n7', label: 'C', type: 'component', componentRef: 'metadata:directory' }, + { type: 'separator', id: 'sep_1', order: 5 }, + { id: 'n8', label: 'G', type: 'group', expanded: true, children: [{ id: 'n9', label: 'X', type: 'url', url: 'https://y' }] }, + ], + })).not.toThrow(); + }); + }); + + describe('app sub-schemas', () => { + it('rejects unknown keys on branding, areas, and context selectors', () => { + expect(unknownKeyIssue(AppSchema, { name: 'app_a', label: 'A', branding: { primary: '#fff' } })!.message) + .toContain('`primary` → `primaryColor`'); + expect(unknownKeyIssue(AppSchema, { + name: 'app_a', label: 'A', areas: [{ id: 'ar', label: 'A', navigation: [], items: [] }], + })!.message).toContain('`items` → `navigation`'); + expect(unknownKeyIssue(AppSchema, { + name: 'app_a', label: 'A', + contextSelectors: [{ id: 'pkg', label: 'P', optionsSource: { endpoint: '/x', url: '/y' } }], + })!.message).toContain('`url` → `endpoint`'); + }); + + it('rejects unknown keys on a navigation contribution', () => { + expect(unknownKeyIssue(NavigationContributionSchema, { + app: 'setup', items: [], targetGroup: 'g', + })!.message).toContain('`targetGroup` → `group`'); + }); + }); +}); diff --git a/packages/spec/src/ui/app.zod.ts b/packages/spec/src/ui/app.zod.ts index bd334f23fd..8df865963b 100644 --- a/packages/spec/src/ui/app.zod.ts +++ b/packages/spec/src/ui/app.zod.ts @@ -5,6 +5,7 @@ import { SnakeCaseIdentifierSchema } from '../shared/identifiers.zod'; import { ExpressionInputSchema } from '../shared/expression.zod'; import { I18nLabelSchema } from './i18n.zod'; import { retiredKey } from '../shared/retired-key'; +import { strictUnknownKeyError } from '../shared/suggestions.zod'; /** * Base Navigation Item Schema @@ -25,6 +26,128 @@ import { retiredKey } from '../shared/retired-key'; import { lazySchema } from '../shared/lazy-schema'; import { MetadataProtectionFields } from '../kernel/metadata-protection.zod'; import { ProtectionSchema } from '../shared/protection.zod'; + +/* + * ── Unknown-key strictness (#4001 app step, PR B) ─────────────────────────── + * + * Every AUTHORING schema in this module is `.strict()`. The app shell is the + * densest hand-authored surface on the platform — a navigation tree is where + * an author (or AI) is most likely to write a key from memory — so a silent + * strip here is the most probable instance of the #3405 trap: the entry + * renders, just not the way it was declared. + * + * The nav-item union is a **discriminated** union on `type`. That is what + * makes strict readable: a plain `z.union` of strict members answers one + * unknown key with an `invalid_union` aggregate naming every branch's + * failure, whereas the discriminated form matches on `type` FIRST and then + * reports a single `unrecognized_keys` issue against that branch alone — + * with an exact path through nested `children`. A mistyped `type` gets its + * own precise "Invalid discriminator value" instead of the same wall. + * (`view.zod.ts` and `widget.zod.ts` set the in-repo precedent.) + * + * Deliberately still OPEN: `PageNavItem.params` / `ComponentNavItem.params` + * (React props passed verbatim to a component) and `ActionNavItem.actionDef. + * params` (action arguments) — those are per-target payloads whose contract + * belongs to the page/component/action, not to the nav item. + * + * PR A (#4142) is the precondition: the seven audit-dead App keys became + * tombstones there, so strictness now guards the real contract rather than + * dead surface (ADR-0049 enforce-or-remove). + */ + +/** Keys every nav-item variant shares (drift-guarded by app.test.ts). */ +const BASE_NAV_ITEM_KEYS = [ + 'id', 'label', 'icon', 'order', 'badge', 'badgeVariant', 'visible', + 'requiredPermissions', 'requiresObject', 'requiresService', 'type', +] as const; + +/** + * Semantic near-misses shared by every nav-item variant. + * + * `visibleWhen` is the load-bearing entry, and the same one #3746 found on + * action params: ADR-0089 made `visibleWhen` the canonical predicate on + * view/page schemas, so an author who learned it there writes it here — and + * before this, borrowing it silently REMOVED the entry's visibility gate, + * rendering a nav item that should have been hidden. A capability gate that + * fails open is the worst shape of the silent-strip bug. + */ +const NAV_ITEM_ALIASES: Readonly> = { + visiblewhen: 'visible', + visibleon: 'visible', + visibility: 'visible', + hidden: 'visible', + title: 'label', + name: 'id', + sort: 'order', + sortorder: 'order', + position: 'order', + permissions: 'requiredPermissions', + requiredpermission: 'requiredPermissions', + requiresobjects: 'requiresObject', + badgecolor: 'badgeVariant', + badgestyle: 'badgeVariant', + // Found by this very gate: the platform's own Account app declared + // `defaultOpen` on three groups (never a schema key), so all three shipped + // COLLAPSED while their author believed they opened by default. + defaultopen: 'expanded', + open: 'expanded', + collapsed: 'expanded', + isopen: 'expanded', +}; + +/** Per-variant payload keys, for the error map's suggestion pool. */ +const NAV_VARIANT_KEYS: Readonly> = { + object: ['objectName', 'viewName', 'recordId', 'recordMode', 'filters', 'children'], + dashboard: ['dashboardName'], + page: ['pageName', 'params'], + url: ['url', 'target'], + report: ['reportName'], + action: ['actionDef'], + component: ['componentRef', 'params'], + group: ['expanded', 'children'], + separator: [], +}; + +/** + * Build the strict error map for one nav-item variant. Each variant gets its + * own so the "did you mean" pool is that variant's real key set — suggesting + * `dashboardName` on a `url` item would be noise, not help. + */ +const navItemUnknownKeyError = (variant: keyof typeof NAV_VARIANT_KEYS) => + strictUnknownKeyError({ + surface: `this \`${variant}\` navigation item`, + knownKeys: [...BASE_NAV_ITEM_KEYS, ...NAV_VARIANT_KEYS[variant]], + aliases: { + ...NAV_ITEM_ALIASES, + // Cross-variant payloads: naming the right key on the wrong `type` is + // the commonest nav mistake, so point at the type that owns it. + ...(variant !== 'object' ? { objectname: 'type: \'object\' (with objectName)' } : {}), + ...(variant !== 'page' ? { pagename: 'type: \'page\' (with pageName)' } : {}), + ...(variant !== 'url' ? { url: 'type: \'url\' (with url)' } : {}), + ...(variant !== 'dashboard' ? { dashboardname: 'type: \'dashboard\' (with dashboardName)' } : {}), + ...(variant !== 'report' ? { reportname: 'type: \'report\' (with reportName)' } : {}), + ...(variant !== 'component' ? { componentref: 'type: \'component\' (with componentRef)' } : {}), + }, + guidance: { + children: + '`children` is only meaningful on a `group` item (or an `object` item nesting its ' + + 'views). Nest entries under `{ type: \'group\', children: [...] }`.', + }, + history: + 'Until #4001 these were dropped silently — the entry still parsed, so a mis-spelled ' + + 'config shipped as a nav item that quietly ignored it (a stripped `visible` renders ' + + 'an entry that should have been gated).', + }); + +const actionDefUnknownKeyError = strictUnknownKeyError({ + surface: "this nav item's action definition", + knownKeys: ['actionName', 'params'], + aliases: { action: 'actionName', name: 'actionName', args: 'params', input: 'params' }, + history: + 'Until #4001 these were dropped silently — the definition still parsed, so clicking ' + + 'the entry dispatched a different action than the author declared.', +}); + const BaseNavItemSchema = z.object({ /** Unique identifier for the item */ id: SnakeCaseIdentifierSchema.describe('Unique identifier for this navigation item (lowercase snake_case)'), @@ -114,7 +237,8 @@ const BaseNavItemSchema = z.object({ * objectName: 'ticket', filters: { owner_id: '{current_user_id}', status: 'open' } } * ``` */ -export const ObjectNavItemSchema = lazySchema(() => BaseNavItemSchema.extend({ +export const ObjectNavItemSchema = lazySchema(() => z.object({ + ...BaseNavItemSchema.shape, type: z.literal('object'), objectName: z.string().describe('Target object name'), viewName: z.string().optional().describe('Default list view to open. Defaults to "all". Ignored when `recordId` is set.'), @@ -153,7 +277,7 @@ export const ObjectNavItemSchema = lazySchema(() => BaseNavItemSchema.extend({ filters: z.record(z.string(), z.string()).optional().describe( 'URL filter conditions — targets the /:objectName/data bare surface via filter[]= params instead of a saved view. Values support template vars {current_user_id}, {current_org_id}. Mutually exclusive with recordId/viewName.', ), -})); +}, { error: navItemUnknownKeyError('object') }).strict()); /** * Correct-by-construction guard (ADR-0053 philosophy): `filters` combined @@ -184,51 +308,58 @@ const objectNavTargetExclusivity = ( * 2. Dashboard Navigation Item * Navigates to a specific dashboard. */ -export const DashboardNavItemSchema = lazySchema(() => BaseNavItemSchema.extend({ +export const DashboardNavItemSchema = lazySchema(() => z.object({ + ...BaseNavItemSchema.shape, type: z.literal('dashboard'), dashboardName: z.string().describe('Target dashboard name'), -})); +}, { error: navItemUnknownKeyError('dashboard') }).strict()); /** * 3. Page Navigation Item * Navigates to a custom UI page/component. */ -export const PageNavItemSchema = lazySchema(() => BaseNavItemSchema.extend({ +export const PageNavItemSchema = lazySchema(() => z.object({ + ...BaseNavItemSchema.shape, type: z.literal('page'), pageName: z.string().describe('Target custom page component name'), + // OPEN by design: the page owns its own param contract. params: z.record(z.string(), z.unknown()).optional().describe('Parameters passed to the page context'), -})); +}, { error: navItemUnknownKeyError('page') }).strict()); /** * 4. URL Navigation Item * Navigates to an external or absolute URL. */ -export const UrlNavItemSchema = lazySchema(() => BaseNavItemSchema.extend({ +export const UrlNavItemSchema = lazySchema(() => z.object({ + ...BaseNavItemSchema.shape, type: z.literal('url'), url: z.string().describe('Target external URL'), target: z.enum(['_self', '_blank']).default('_self').describe('Link target window'), -})); +}, { error: navItemUnknownKeyError('url') }).strict()); /** * 5. Report Navigation Item * Navigates to a specific report. */ -export const ReportNavItemSchema = lazySchema(() => BaseNavItemSchema.extend({ +export const ReportNavItemSchema = lazySchema(() => z.object({ + ...BaseNavItemSchema.shape, type: z.literal('report'), reportName: z.string().describe('Target report name'), -})); +}, { error: navItemUnknownKeyError('report') }).strict()); /** * 6. Action Navigation Item * Triggers an action (e.g. opening a flow, running a script, or launching a screen action). */ -export const ActionNavItemSchema = lazySchema(() => BaseNavItemSchema.extend({ +export const ActionNavItemSchema = lazySchema(() => z.object({ + ...BaseNavItemSchema.shape, type: z.literal('action'), actionDef: z.object({ actionName: z.string().describe('Action machine name to execute'), + // OPEN by design: the action owns its own param contract. params: z.record(z.string(), z.unknown()).optional().describe('Parameters passed to the action'), - }).describe('Action definition to execute when clicked'), -})); + }, { error: actionDefUnknownKeyError }).strict().describe('Action definition to execute when clicked'), +}, { error: navItemUnknownKeyError('action') }).strict()); /** * 7. Component Navigation Item @@ -249,22 +380,25 @@ export const ActionNavItemSchema = lazySchema(() => BaseNavItemSchema.extend({ * componentRef: 'metadata:resource', params: { type: 'object' } } * ``` */ -export const ComponentNavItemSchema = lazySchema(() => BaseNavItemSchema.extend({ +export const ComponentNavItemSchema = lazySchema(() => z.object({ + ...BaseNavItemSchema.shape, type: z.literal('component'), componentRef: z.string().describe('Component registry key (e.g. "metadata:directory")'), + // OPEN by design: props are the component's own contract. params: z.record(z.string(), z.unknown()).optional().describe('Props passed to the component'), -})); +}, { error: navItemUnknownKeyError('component') }).strict()); /** * 8. Group Navigation Item * A container for child navigation items (Sub-menu). * Does not perform navigation itself. */ -export const GroupNavItemSchema = lazySchema(() => BaseNavItemSchema.extend({ +export const GroupNavItemSchema = lazySchema(() => z.object({ + ...BaseNavItemSchema.shape, type: z.literal('group'), expanded: z.boolean().default(false).describe('Default expansion state in sidebar'), // children property is added in the recursive definition below -})); +}, { error: navItemUnknownKeyError('group') }).strict()); /** * 9. Separator Navigation Item @@ -276,17 +410,23 @@ const SeparatorNavItemSchema = lazySchema(() => z.object({ type: z.literal('separator'), id: SnakeCaseIdentifierSchema.optional().describe('Optional id for the separator'), order: z.number().optional().describe('Sort order within the same level (lower = first)'), -})); +}, { error: navItemUnknownKeyError('separator') }).strict()); /** * Recursive Union of all navigation item types. * Allows constructing an unlimited-depth navigation tree. */ export const NavigationItemSchema: z.ZodType = z.lazy(() => - z.union([ + // DISCRIMINATED on `type` (#4001 PR B). With `.strict()` members a plain + // union would answer one unknown key with an `invalid_union` aggregate + // listing all nine branches' failures; discriminating on `type` first means + // the author gets a single `unrecognized_keys` issue against the branch they + // actually wrote, at an exact path (`navigation.0.children.2`), and a + // mistyped `type` gets "Invalid discriminator value" instead of that wall. + z.discriminatedUnion('type', [ ObjectNavItemSchema.extend({ children: z.array(NavigationItemSchema).optional().describe('Child navigation items (e.g. specific views)'), - }).superRefine(objectNavTargetExclusivity), + }).strict().superRefine(objectNavTargetExclusivity), DashboardNavItemSchema, PageNavItemSchema, UrlNavItemSchema, @@ -296,8 +436,11 @@ export const NavigationItemSchema: z.ZodType = z.lazy(() => SeparatorNavItemSchema, GroupNavItemSchema.extend({ children: z.array(NavigationItemSchema).describe('Child navigation items'), - }) - ]) + }).strict(), + // The members are lazySchema Proxies and a superRefine-wrapped variant, so + // the array is widened for the discriminator-typed overload; runtime + // discrimination works on all of them (asserted in app.test.ts). + ] as unknown as readonly [z.ZodObject, ...z.ZodObject[]]) ); /** @@ -331,7 +474,16 @@ export const NavigationContributionSchema = lazySchema(() => z.object({ group: SnakeCaseIdentifierSchema.optional().describe('Target group nav-item id to append into (e.g. "group_integrations"); omit to append at the app top level'), priority: z.number().int().min(0).default(200).describe('Merge priority within the target group — lower applied first (matches object extender priority)'), items: z.array(NavigationItemSchema).describe('Navigation items contributed into the target app/group'), -}).describe('A navigation contribution: a package injecting nav items into an app it does not own (ADR-0029 D7)')); +}, { + error: strictUnknownKeyError({ + surface: 'this navigation contribution', + knownKeys: ['app', 'group', 'priority', 'items'], + aliases: { targetapp: 'app', appname: 'app', targetgroup: 'group', groupid: 'group', order: 'priority', navigation: 'items' }, + history: + 'Until #4001 these were dropped silently — the contribution still parsed, so a ' + + 'package injected its menu into the wrong place, or nowhere.', + }), +}).strict().describe('A navigation contribution: a package injecting nav items into an app it does not own (ADR-0029 D7)')); export type NavigationContribution = z.infer; /** @@ -343,7 +495,16 @@ export const AppBrandingSchema = lazySchema(() => z.object({ accentColor: z.string().optional().describe('Accent color hex code (highlights, active states). Declared to match the objectui ConsoleLayout read of branding.accentColor (inverse-drift fix, liveness audit #1878/#1891/#1894).'), logo: z.string().optional().describe('Custom logo URL for this app'), favicon: z.string().optional().describe('Custom favicon URL for this app'), -})); +}, { + error: strictUnknownKeyError({ + surface: "this app's branding block", + knownKeys: ['primaryColor', 'accentColor', 'logo', 'favicon'], + aliases: { primary: 'primaryColor', accent: 'accentColor', color: 'primaryColor', logourl: 'logo', icon: 'favicon', theme: 'primaryColor' }, + history: + 'Until #4001 these were dropped silently — branding still parsed, so a theme the ' + + 'author set never reached the shell.', + }), +}).strict()); /** * Navigation Area Schema @@ -397,7 +558,16 @@ export const NavigationAreaSchema = lazySchema(() => z.object({ /** Navigation items within this area */ navigation: z.array(NavigationItemSchema).describe('Navigation items within this area'), -})); +}, { + error: strictUnknownKeyError({ + surface: 'this navigation area', + knownKeys: ['id', 'label', 'icon', 'order', 'description', 'visible', 'requiredPermissions', 'navigation'], + aliases: { visiblewhen: 'visible', visibleon: 'visible', title: 'label', name: 'id', sort: 'order', permissions: 'requiredPermissions', items: 'navigation', children: 'navigation' }, + history: + 'Until #4001 these were dropped silently — the area still parsed, so its gating or ' + + 'ordering was quietly ignored.', + }), +}).strict()); /** * App Context Selector Schema @@ -481,8 +651,26 @@ export const AppContextSelectorSchema = lazySchema(() => z.object({ .describe('Comparison operator: eq | ne | in | nin'), value: z.union([z.string(), z.array(z.string())]) .describe('Comparison value (string for eq/ne, string[] for in/nin)'), - })).optional().describe('Predicates (AND) each option row must satisfy'), - }).describe('Option data source'), + }, { + error: strictUnknownKeyError({ + surface: 'this context-selector option filter', + knownKeys: ['key', 'op', 'value'], + aliases: { field: 'key', path: 'key', operator: 'op', values: 'value' }, + history: + 'Until #4001 these were dropped silently — the predicate still parsed, so the ' + + 'option list was not narrowed the way the author declared.', + }), + }).strict()).optional().describe('Predicates (AND) each option row must satisfy'), + }, { + error: strictUnknownKeyError({ + surface: "this context selector's options source", + knownKeys: ['endpoint', 'valueKey', 'labelKey', 'filter'], + aliases: { url: 'endpoint', path: 'endpoint', value: 'valueKey', label: 'labelKey', filters: 'filter', where: 'filter' }, + history: + 'Until #4001 these were dropped silently — the source still parsed, so the ' + + 'dropdown resolved its options from a different shape than declared.', + }), + }).strict().describe('Option data source'), /** Whether to prepend an "All" option that clears the scope. */ includeAll: z.boolean().default(true).describe('Prepend an "All" option that clears the scope'), @@ -497,7 +685,16 @@ export const AppContextSelectorSchema = lazySchema(() => z.object({ /** Where the dropdown is rendered. */ placement: z.enum(['sidebar_header', 'topbar']).default('sidebar_header') .describe('Render location in the app chrome'), -})); +}, { + error: strictUnknownKeyError({ + surface: 'this app context selector', + knownKeys: ['id', 'label', 'icon', 'optionsSource', 'includeAll', 'allValue', 'persist', 'placement'], + aliases: { name: 'id', title: 'label', source: 'optionsSource', options: 'optionsSource', showall: 'includeAll', location: 'placement' }, + history: + 'Until #4001 these were dropped silently — the selector still parsed, so its scope ' + + 'variable behaved differently than declared.', + }), +}).strict()); export type AppContextSelector = z.infer; @@ -542,6 +739,59 @@ export type AppContextSelector = z.infer; * ] * } */ +/** Keys {@link AppSchema} declares (drift-guarded by app.test.ts). */ +const APP_KEYS = [ + 'name', 'label', 'description', 'icon', 'branding', 'active', 'isDefault', + 'hidden', 'navigation', 'areas', 'contextSelectors', 'homePageId', + 'requiredPermissions', 'defaultAgent', 'protection', + // ADR-0010 runtime protection envelope (MetadataProtectionFields spread). + '_lock', '_lockReason', '_lockSource', '_provenance', '_packageId', + '_packageVersion', '_lockDocsUrl', + // Tombstoned in PR A (#4142) — declared so the prescription, not a bare + // "unrecognized key", is what an upgrading author sees. + 'version', 'aria', 'objects', 'apis', 'sharing', 'embed', 'mobileNavigation', +] as const; + +const appUnknownKeyError = strictUnknownKeyError({ + surface: 'this app', + knownKeys: APP_KEYS, + aliases: { + title: 'label', + nav: 'navigation', + menu: 'navigation', + menus: 'navigation', + items: 'navigation', + sidebar: 'navigation', + tabs: 'navigation', + sections: 'areas', + groups: 'areas', + permissions: 'requiredPermissions', + home: 'homePageId', + homepage: 'homePageId', + landingpage: 'homePageId', + agent: 'defaultAgent', + logo: 'branding', + theme: 'branding', + enabled: 'active', + default: 'isDefault', + selectors: 'contextSelectors', + }, + guidance: { + pages: + '`pages` is not an App field — a page is its own metadata record; reference it from ' + + "navigation with `{ type: 'page', pageName: '' }`.", + views: + '`views` is not an App field — views belong to their object (`listViews`); reference ' + + "one from navigation with `{ type: 'object', objectName, viewName }`.", + flows: + '`flows` is not an App field — flows are top-level stack metadata ' + + '(`defineStack({ flows })`), not app-scoped.', + }, + history: + 'Until #4001 these were dropped silently — the app still parsed, so navigation or ' + + 'gating the author declared never reached the shell.', +}); + export const AppSchema = lazySchema(() => z.object({ /** Machine name (id) */ name: SnakeCaseIdentifierSchema.describe('App unique machine name (lowercase snake_case)'), @@ -740,7 +990,7 @@ export const AppSchema = lazySchema(() => z.object({ // ADR-0010 — runtime protection envelope (internal — set by loader). ...MetadataProtectionFields, -})); +}, { error: appUnknownKeyError }).strict()); /** * App Factory Helper