fix(types): chain the spec's objectNavTargetExclusivity in NavigationItemSchema (objectui#8563)

.changeset/8563-nav-target-exclusivity-chained.md

@@ -0,0 +1,42 @@
+---
+"@object-ui/types": minor
+---
+
+`NavigationItemSchema` chains the spec's own `objectNavTargetExclusivity` on the
+`type: 'object'` arm, instead of accepting target combinations the platform refuses
+(objectui#8563).
+
+This schema is hand-written rather than derived from a spec `.shape`, so nothing carried
+the spec's checks across it. An object nav entry declaring both `filters` and `recordId`
+— or both `runAction` and `recordId` — parsed clean here and was then refused by
+`@objectstack/spec`, i.e. at publish. The drift surfaced only at the most expensive point
+to find it, which is the tolerant-consumer shape this repo's contract rule forbids.
+
+The rule is CHAINED, not restated. A local copy of its body passes every case on the day
+it is written and starts drifting the day the spec's own rule moves — the same defect one
+layer down. `../__tests__/nav-target-exclusivity-8563.test.ts` compares this door's issues
+byte for byte against the exported function driven directly, and parses the mirror's source
+to refuse a local re-declaration of the name.
+
+**Breaking for authors, and shipped as `minor` deliberately.** The accept set narrows:
+documents combining `filters` with `recordId` / `viewName`, or `runAction` with `recordId`,
+stop validating here. Anything writing one was authoring metadata the platform already
+refused at publish — the same judgement, and the same bump, as the `formats` no longer
+admitting `'pdf'` entry in 17.5.0. This repo's fixed release group tracks `@objectstack`'s
+major, so objectui's own breaking changes ship as `minor` with the break spelled out here
+(AGENTS.md §版本号策略, mechanically enforced by `scripts/check-changeset-no-major.mjs`).
+
+⚠️ The rule is deliberately NOT pairwise-exclusive, and that asymmetry is preserved rather
+than tidied: `recordId` + `viewName` stays TOLERATED, and `runAction` composes with
+`viewName` or `filters` — it is refused with `recordId` only. Six negative controls pin
+the neighbours that must still parse.
+
+Two `.describe()` strings and the `NavigationItem` interface doc taught a
+`Precedence: recordId → filters → viewName` that no longer resolves anything — the
+combination is refused, so an author following the sentence got a rejection. They now read
+`Mutually exclusive with recordId/viewName.`, matching the spec's own describe.
+
+`@objectstack/spec`'s declared floor moves `^17.3.0` → `^17.4.0`: 17.4.0 is the first
+published version that EXPORTS the rule (bisected across the published 17.x line against
+each version's own tarball, not against workspace resolution). The published artifact now
+references the symbol, so `check:spec-floors` requires the floor to carry it.

packages/types/package.json

@@ -93,7 +93,7 @@
     "directory": "packages/types"
   },
   "dependencies": {
-    "@objectstack/spec": "^17.3.0",
+    "@objectstack/spec": "^17.4.0",
     "zod": "^4.4.3"
   },
   "devDependencies": {

packages/types/src/__tests__/imported-defaults-8317.test.ts

@@ -75,6 +75,7 @@ import {
   I18nLabelSchema as SpecI18nLabelSchema,
   ChartAggregateSchema as SpecChartAggregateSchema,
   ChartDrillDownSchema as SpecChartDrillDownSchema,
+  objectNavTargetExclusivity,
 } from '@objectstack/spec/ui';
 import { SelectOptionSchema as SpecSelectOptionSchema } from '@objectstack/spec/data';
 import { stripImportedDefaults } from '../zod/imported-defaults.js';
@@ -377,8 +378,8 @@ describe('the import boundary strips every imported default (objectui#8317)', ()
    * `@objectstack/spec` binding inside a mirror must be the direct argument of
    * `stripImportedDefaults(…)`.
    *
-   * Two kinds of read are declared exceptions, and they are enumerated here
-   * rather than pattern-matched, so adding a third is an edit to this list:
+   * Three kinds of read are declared exceptions, and they are enumerated here
+   * rather than pattern-matched, so adding a fourth is an edit to this list:
    *
    *  - a value VOCABULARY — `SpecListViewTypeEnum` / `ViewKindEnum`, which
    *    unwrap the spec's own `.default('grid')` to reach its enum. A set of
@@ -388,6 +389,11 @@ describe('the import boundary strips every imported default (objectui#8317)', ()
    *    and throw.
    *  - a TYPE position, where there is no runtime schema to strip and the
    *    declared type is unchanged by the strip anyway.
+   *  - a chained REFINEMENT (objectui#8563) — a spec check FUNCTION such as
+   *    `objectNavTargetExclusivity`, which a mirror mounts on its own schema.
+   *    There is no Zod graph to walk and no default to remove: the whole effect
+   *    of a refinement is `ctx.addIssue`, so it cannot write a value into a
+   *    parsed document, which is the only thing this boundary is about.
    */
   describe('every `@objectstack/spec` value read in the mirrors goes through the boundary', () => {
     /** `<file>:<enclosing const>` for each read that is allowed to stay raw. */
@@ -396,6 +402,21 @@ describe('the import boundary strips every imported default (objectui#8317)', ()
       'objectql.zod.ts:ViewKindEnum',
     ]);
 
+    /**
+     * Spec CHECK FUNCTIONS a mirror chains, keyed by binding name rather than by
+     * owning const: a refinement is read inside whichever schema mounts it, and
+     * the same rule may be mounted on more than one. Chaining these is the
+     * POINT rather than a tolerated exception — a mirror that restated the rule
+     * body instead would drift from the spec's the day the spec's own moved,
+     * which is the defect objectui#8563 closed.
+     *
+     * Held as name → binding so the assertion below can check each entry really
+     * is a function: a schema must not reach this list merely by being listed.
+     */
+    const REFINEMENT_EXCEPTIONS = new Map<string, unknown>([
+      ['objectNavTargetExclusivity', objectNavTargetExclusivity],
+    ]);
+
     const isSpecModule = (m: string): boolean =>
       m === '@objectstack/spec' || m.startsWith('@objectstack/spec/');
 
@@ -462,7 +483,8 @@ describe('the import boundary strips every imported default (objectui#8317)', ()
     it('no value read bypasses `stripImportedDefaults`', () => {
       const offenders = reads
         .filter((r) => r.kind === 'value' && !r.wrapped)
-        .filter((r) => !VOCABULARY_EXCEPTIONS.has(`${r.file}:${r.owner}`));
+        .filter((r) => !VOCABULARY_EXCEPTIONS.has(`${r.file}:${r.owner}`))
+        .filter((r) => !REFINEMENT_EXCEPTIONS.has(r.name));
       expect(
         offenders.map((r) => `${r.file}:${r.line} ${r.name} (in \`${r.owner ?? '<top level>'}\`)`),
         'an `@objectstack/spec` schema crosses into a mirror without the objectui#8317 import ' +
@@ -486,10 +508,23 @@ describe('the import boundary strips every imported default (objectui#8317)', ()
       }
     });
 
+    it('every declared refinement exception is a LIVE FUNCTION, not a schema in disguise', () => {
+      expect(REFINEMENT_EXCEPTIONS.size, 'the list is empty — delete it rather than leave a hole').toBeGreaterThan(0);
+      for (const [name, binding] of REFINEMENT_EXCEPTIONS) {
+        expect(typeof binding, `${name} is not a function, so it does not belong in this list`).toBe('function');
+        expect(
+          '_zod' in Object(binding),
+          `${name} carries Zod internals — it is a schema, and a schema crosses the boundary`,
+        ).toBe(false);
+        const matching = reads.filter((r) => r.name === name && r.kind === 'value');
+        expect(matching.length, `declared exception ${name} matches no read — delete it`).toBeGreaterThan(0);
+      }
+    });
+
     it('every symbol the mirrors import is covered by the differential above', () => {
       const differential = new Set(IMPORTED.map(([n]) => n));
       const missing = [...new Set(reads.map((r) => r.name.replace(/^Spec/, '')))]
-        .filter((n) => !differential.has(n));
+        .filter((n) => !differential.has(n) && !REFINEMENT_EXCEPTIONS.has(n));
       expect(
         missing,
         'a schema imported by a mirror is not in this file\'s `IMPORTED` list, so nothing measures ' +

packages/types/src/__tests__/nav-target-exclusivity-8563.test.ts

@@ -0,0 +1,197 @@
+/**
+ * ObjectUI
+ * Copyright (c) 2024-present ObjectStack Inc.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ */
+
+/**
+ * `NavigationItemSchema` CHAINS the spec's target-exclusivity rule (objectui#8563).
+ *
+ * `packages/types/src/zod/app.zod.ts` writes this schema by hand rather than
+ * deriving it from a spec `.shape`, so no mechanism carried the spec's own
+ * checks across: an object nav entry declaring both `filters` and `recordId`
+ * parsed clean HERE and was refused by `@objectstack/spec`, i.e. by the publish
+ * door — the drift only surfaced where it was most expensive to find.
+ *
+ * The fix is to call the spec's exported `objectNavTargetExclusivity`, and the
+ * distinction this file exists to police is CHAIN vs COPY. A hand-copy of the
+ * rule body passes every accept/refuse case on the day it is written and starts
+ * drifting the day the spec's own rule moves — which is the defect above,
+ * re-created one layer down. So two of the assertions below are about identity
+ * rather than behaviour:
+ *
+ *  - the door's issues are compared BYTE FOR BYTE against the same function
+ *    driven directly, so a reworded local copy fails even when it refuses the
+ *    same set;
+ *  - the mirror's source is parsed, so a local re-declaration of the name fails
+ *    even if it happened to produce identical bytes.
+ *
+ * ⚠️ The rule is deliberately NOT pairwise-exclusive over the target fields, and
+ * the six negative controls are the half that keeps a "tighten it everywhere"
+ * edit from passing: `recordId` + `viewName` is TOLERATED, and `runAction` is
+ * refused with `recordId` ONLY — it composes with `viewName` or `filters`. Both
+ * asymmetries are the spec's on purpose; they are re-derived here from the
+ * installed rule, not from prose.
+ */
+
+import { describe, it, expect } from 'vitest';
+import { readFileSync } from 'node:fs';
+import { dirname, join } from 'node:path';
+import { fileURLToPath } from 'node:url';
+import ts from 'typescript';
+import type { z } from 'zod';
+import { objectNavTargetExclusivity } from '@objectstack/spec/ui';
+import { NavigationItemSchema } from '../zod/app.zod.js';
+
+const MIRROR = join(dirname(fileURLToPath(import.meta.url)), '..', 'zod', 'app.zod.ts');
+const RULE = 'objectNavTargetExclusivity';
+
+/** A valid object entry; each case below adds ONLY the target fields it names. */
+const BASE = { id: 'nav_tickets', type: 'object', label: 'Tickets', objectName: 'ticket' } as const;
+
+const parse = (targets: Record<string, unknown>) => NavigationItemSchema.safeParse({ ...BASE, ...targets });
+
+/** `{ code, path, message }` for each issue the DOOR raised, order preserved. */
+const doorIssues = (targets: Record<string, unknown>): Array<Record<string, unknown>> => {
+  const r = parse(targets);
+  if (r.success) return [];
+  return r.error.issues.map((i) => ({ code: i.code, path: [...i.path], message: i.message }));
+};
+
+/**
+ * The rule's OWN declared parameter surface, read off the export rather than
+ * restated: `{ filters?, recordId?, viewName?, runAction? }`, all `unknown`. It
+ * is a weak type, so the full nav entry is widened into it deliberately — a
+ * nav item is a superset of the four fields the rule reads.
+ */
+type RuleInput = Parameters<typeof objectNavTargetExclusivity>[0];
+
+/** …and for each issue the SPEC's exported rule raises, driven directly. */
+const specRuleIssues = (targets: Record<string, unknown>): Array<Record<string, unknown>> => {
+  const issues: Array<Record<string, unknown>> = [];
+  const ctx = { addIssue: (i: Record<string, unknown>) => issues.push(i) } as unknown as z.RefinementCtx;
+  objectNavTargetExclusivity({ ...BASE, ...targets } as RuleInput, ctx);
+  return issues.map((i) => ({ code: i.code, path: [...(i.path as unknown[])], message: i.message as string }));
+};
+
+describe('the object arm refuses the ambiguous landings (objectui#8563)', () => {
+  it('refuses `filters` + `recordId` at `filters`, with code custom', () => {
+    const issues = doorIssues({ filters: { status: 'open' }, recordId: 'rec_1' });
+    expect(issues).toHaveLength(1);
+    expect(issues[0].code).toBe('custom');
+    expect(issues[0].path).toEqual(['filters']);
+  });
+
+  it('refuses `filters` + `viewName` at `filters`, with code custom', () => {
+    const issues = doorIssues({ filters: { status: 'open' }, viewName: 'open_tickets' });
+    expect(issues).toHaveLength(1);
+    expect(issues[0].code).toBe('custom');
+    expect(issues[0].path).toEqual(['filters']);
+  });
+
+  it('refuses `runAction` + `recordId` at `runAction`, with code custom', () => {
+    const issues = doorIssues({ runAction: 'create_ticket', recordId: 'rec_1' });
+    expect(issues).toHaveLength(1);
+    expect(issues[0].code).toBe('custom');
+    expect(issues[0].path).toEqual(['runAction']);
+  });
+
+  it('reaches the rule through a nested `children` entry too, not only at the root', () => {
+    const r = NavigationItemSchema.safeParse({
+      id: 'grp', type: 'group', label: 'Group',
+      children: [{ ...BASE, filters: { status: 'open' }, recordId: 'rec_1' }],
+    });
+    expect(r.success).toBe(false);
+    expect(r.success ? [] : r.error.issues.map((i) => [...i.path])).toContainEqual(['children', 0, 'filters']);
+  });
+});
+
+describe('the neighbours the rule deliberately tolerates still parse', () => {
+  // ⛔ Do not "simplify" this into pairwise exclusivity. Every row is a landing
+  // the spec accepts on purpose; a row flipping to refused is a narrowing this
+  // repo invented, not one it inherited.
+  const CONTROLS: Array<readonly [string, Record<string, unknown>]> = [
+    ['filters alone', { filters: { status: 'open' } }],
+    ['recordId alone', { recordId: 'rec_1' }],
+    ['viewName alone', { viewName: 'open_tickets' }],
+    ['recordId + viewName (tolerated legacy pair)', { recordId: 'rec_1', viewName: 'open_tickets' }],
+    ['runAction + filters', { runAction: 'create_ticket', filters: { status: 'open' } }],
+    ['runAction + viewName', { runAction: 'create_ticket', viewName: 'open_tickets' }],
+  ];
+
+  it.each(CONTROLS)('accepts %s', (_label, targets) => {
+    const r = parse(targets);
+    expect(r.success ? [] : r.error.issues.map((i) => `${[...i.path].join('.')}: ${i.message}`)).toEqual([]);
+    expect(r.success).toBe(true);
+  });
+
+  it('the controls are not vacuous — the rule itself raises nothing for any of them', () => {
+    // Guards the comparison in the identity test below from being empty==empty.
+    for (const [label, targets] of CONTROLS) {
+      expect(specRuleIssues(targets), `the spec rule refused the control "${label}"`).toEqual([]);
+    }
+  });
+});
+
+describe('the rule is CHAINED, not copied', () => {
+  it('the published export is a live two-argument function', () => {
+    expect(typeof objectNavTargetExclusivity).toBe('function');
+    expect(objectNavTargetExclusivity.name).toBe(RULE);
+    expect(objectNavTargetExclusivity.length).toBe(2);
+  });
+
+  it("the door's issues are the spec rule's own bytes, not a restatement", () => {
+    for (const targets of [
+      { filters: { status: 'open' }, recordId: 'rec_1' },
+      { filters: { status: 'open' }, viewName: 'open_tickets' },
+      { runAction: 'create_ticket', recordId: 'rec_1' },
+      { filters: { status: 'open' }, recordId: 'rec_1', runAction: 'create_ticket' },
+    ]) {
+      const fromRule = specRuleIssues(targets);
+      expect(fromRule.length, 'the instrument saw no issue at all').toBeGreaterThan(0);
+      expect(doorIssues(targets)).toEqual(fromRule);
+    }
+  });
+
+  it('the mirror imports the rule from `@objectstack/spec/ui` and declares no local copy', () => {
+    const text = readFileSync(MIRROR, 'utf8');
+    const sf = ts.createSourceFile('app.zod.ts', text, ts.ScriptTarget.Latest, true, ts.ScriptKind.TS);
+
+    let importedFrom: string | null = null;
+    const localDeclarations: string[] = [];
+    let calls = 0;
+
+    const visit = (n: ts.Node): void => {
+      if (ts.isImportDeclaration(n) && ts.isStringLiteral(n.moduleSpecifier)) {
+        const named = n.importClause?.namedBindings;
+        if (named && ts.isNamedImports(named)) {
+          for (const el of named.elements) {
+            if ((el.propertyName ?? el.name).text === RULE) importedFrom = n.moduleSpecifier.text;
+          }
+        }
+      }
+      // A local re-declaration is the copy this test exists to refuse.
+      if (ts.isFunctionDeclaration(n) && n.name?.text === RULE) localDeclarations.push('function');
+      if (ts.isVariableDeclaration(n) && ts.isIdentifier(n.name) && n.name.text === RULE) localDeclarations.push('const');
+      if (ts.isCallExpression(n) && ts.isIdentifier(n.expression) && n.expression.text === RULE) calls += 1;
+      ts.forEachChild(n, visit);
+    };
+    visit(sf);
+
+    expect(importedFrom).toBe('@objectstack/spec/ui');
+    expect(localDeclarations).toEqual([]);
+    expect(calls, 'the mirror imports the rule but never calls it').toBeGreaterThan(0);
+  });
+
+  it('no `.describe()` in the mirror still teaches the precedence the rule refuses', () => {
+    // The sentence "Precedence: recordId -> filters -> viewName" was copied from
+    // a spec docblock the spec itself corrected: no precedence resolves these
+    // combinations, they are refused. A describe that teaches one is a trap for
+    // whoever authors against it.
+    const text = readFileSync(MIRROR, 'utf8');
+    expect(text).not.toMatch(/Precedence:\s*recordId/i);
+    expect(text).toContain('Mutually exclusive with recordId/viewName.');
+  });
+});

packages/types/src/__tests__/navigation-spec-parity.test.ts

@@ -28,12 +28,19 @@
  *  - `{ type: 'separator' }` — spec-valid, rejected for missing id/label.
  *
  * Deliberately NOT modelled: the spec expresses navigation as a discriminated
- * union of nine variants, each with its target field required and a
- * `superRefine` exclusivity rule. objectui keeps one flat, all-optional shape,
- * so it accepts items the spec would reject (e.g. `type: 'object'` with no
- * `objectName`). Converging on the union is a breaking change for every
- * consumer that reads fields off `NavigationItem` without narrowing — tracked
- * separately, not smuggled in here.
+ * union of nine variants, each with its target field required. objectui keeps
+ * one flat, all-optional shape, so it accepts items the spec would reject (e.g.
+ * `type: 'object'` with no `objectName`). Converging on the union is a breaking
+ * change for every consumer that reads fields off `NavigationItem` without
+ * narrowing — tracked separately, not smuggled in here.
+ *
+ * ⚠️ The object arm's `superRefine` exclusivity rule is the one part of that
+ * paragraph that no longer holds: objectui#8563 CHAINS the spec's exported
+ * `objectNavTargetExclusivity` on `type: 'object'`, so `filters` combined with
+ * `recordId` / `viewName`, and `runAction` combined with `recordId`, are refused
+ * here as well. That narrowing and the neighbours it deliberately leaves alone
+ * are pinned in `./nav-target-exclusivity-8563.test.ts`, not here — this file
+ * still measures the vocabulary gaps only.
  */
 
 import { describe, it, expect } from 'vitest';