From f5f70bab4f70848343829499c5106905d96f2973 Mon Sep 17 00:00:00 2001 From: os-warren Date: Thu, 10 Sep 2026 21:00:21 +0000 Subject: [PATCH] fix(types): chain the spec's objectNavTargetExclusivity in NavigationItemSchema (objectui#8563) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `NavigationItemSchema` is hand-written rather than derived from a spec `.shape`, so nothing carried the spec's own checks across it: an object nav entry declaring both `filters` and `recordId` — or both `runAction` and `recordId` — parsed clean at this door and was refused by `@objectstack/spec` at publish. The rule is CHAINED, not restated. A local copy passes every case the day it is written and drifts the day the spec's own rule moves, which is this same defect one layer down. The pin 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. The two asymmetries the spec keeps on purpose are preserved and pinned by six negative controls: `recordId` + `viewName` stays tolerated, and `runAction` composes with `viewName` or `filters` — it is refused with `recordId` only. Two `.describe()` strings and the `NavigationItem` interface doc taught a `Precedence: recordId -> filters -> viewName` that resolves nothing, since the combination is refused; they now match the spec's own describe. The `@objectstack/spec` 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 rather than workspace resolution. `imported-defaults-8317.test.ts` gains a third declared exception kind — a chained spec refinement, which is a function with no Zod graph to strip and no accept set to differentiate — with a guard that each entry really is a function. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Jmxdo7bmeqCQHLSfmLVX9w --- .../8563-nav-target-exclusivity-chained.md | 42 ++++ packages/types/package.json | 2 +- .../__tests__/imported-defaults-8317.test.ts | 43 +++- .../nav-target-exclusivity-8563.test.ts | 197 ++++++++++++++++++ .../__tests__/navigation-spec-parity.test.ts | 19 +- packages/types/src/app.ts | 13 +- packages/types/src/zod/app.zod.ts | 29 ++- pnpm-lock.yaml | 2 +- 8 files changed, 329 insertions(+), 18 deletions(-) create mode 100644 .changeset/8563-nav-target-exclusivity-chained.md create mode 100644 packages/types/src/__tests__/nav-target-exclusivity-8563.test.ts diff --git a/.changeset/8563-nav-target-exclusivity-chained.md b/.changeset/8563-nav-target-exclusivity-chained.md new file mode 100644 index 0000000000..5ba5f1e2b9 --- /dev/null +++ b/.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. diff --git a/packages/types/package.json b/packages/types/package.json index f9c5e84e0a..c9d4275d62 100644 --- a/packages/types/package.json +++ b/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": { diff --git a/packages/types/src/__tests__/imported-defaults-8317.test.ts b/packages/types/src/__tests__/imported-defaults-8317.test.ts index 9f26e53ca2..9fb6da27bc 100644 --- a/packages/types/src/__tests__/imported-defaults-8317.test.ts +++ b/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', () => { /** `:` 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([ + ['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 ?? ''}\`)`), '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 ' + diff --git a/packages/types/src/__tests__/nav-target-exclusivity-8563.test.ts b/packages/types/src/__tests__/nav-target-exclusivity-8563.test.ts new file mode 100644 index 0000000000..1d04401fef --- /dev/null +++ b/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) => NavigationItemSchema.safeParse({ ...BASE, ...targets }); + +/** `{ code, path, message }` for each issue the DOOR raised, order preserved. */ +const doorIssues = (targets: Record): Array> => { + 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[0]; + +/** …and for each issue the SPEC's exported rule raises, driven directly. */ +const specRuleIssues = (targets: Record): Array> => { + const issues: Array> = []; + const ctx = { addIssue: (i: Record) => 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]> = [ + ['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.'); + }); +}); diff --git a/packages/types/src/__tests__/navigation-spec-parity.test.ts b/packages/types/src/__tests__/navigation-spec-parity.test.ts index 4120e9c6b2..b4f30a4ab6 100644 --- a/packages/types/src/__tests__/navigation-spec-parity.test.ts +++ b/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'; diff --git a/packages/types/src/app.ts b/packages/types/src/app.ts index b4010c1e67..40a3f0e13e 100644 --- a/packages/types/src/app.ts +++ b/packages/types/src/app.ts @@ -140,7 +140,11 @@ export interface NavigationItem { * (`{current_user_id}`, `{current_org_id}`); entries whose template can't * be resolved are dropped from the URL. * - * Precedence within `type: 'object'`: `recordId` → `filters` → `viewName`. + * Mutually exclusive with `recordId` / `viewName` (objectui#8563): the + * combination is REFUSED by `NavigationItemSchema`, which chains the spec's + * own `objectNavTargetExclusivity`. There is deliberately no precedence to + * resolve it with — an entry picks ONE landing, and the alternative is a + * validator that silently ignores two of the three fields an author wrote. * * Shape derived from the spec's object-nav variant (#3177). */ @@ -154,9 +158,10 @@ export interface NavigationItem { * welcome-page CTA that should land the user IN the create dialog rather than * on the list, hunting for a second button. * - * Landing surface only: `runAction` describes the LIST surface, so it is - * ignored when `recordId` wins the precedence chain and the entry resolves to - * a record detail page instead. `NavigationRenderer.resolveHref` encodes it + * Landing surface only: `runAction` describes the LIST surface, so combining + * it with `recordId` is REFUSED by `NavigationItemSchema` (objectui#8563) — a + * record detail page has no list toolbar for the action to run on. It composes + * with `viewName` or `filters`. `NavigationRenderer.resolveHref` encodes it * as the reserved `?runAction=` search param — {@link NAV_RUN_ACTION_PARAM} * in `@object-ui/layout` is that param name's ONE definition, and the list * toolbar reads it back through the same constant. diff --git a/packages/types/src/zod/app.zod.ts b/packages/types/src/zod/app.zod.ts index 1bdb828b3a..6bd61d6544 100644 --- a/packages/types/src/zod/app.zod.ts +++ b/packages/types/src/zod/app.zod.ts @@ -21,6 +21,13 @@ import { AppSchema as SpecAppSchema, AppContextSelectorSchema as SpecAppContextSelectorSchema, NavigationAreaSchema as SpecNavigationAreaSchema, + // ⚠️ NOT a crossing, so it stays RAW — see THE IMPORT BOUNDARY below. This is + // the spec's own refinement FUNCTION, not a schema: `stripImportedDefaults` + // walks a Zod graph and a refinement has none, and a check that only calls + // `ctx.addIssue` cannot write a default into an author's document. Declared + // as such in `../__tests__/imported-defaults-8317.test.ts` rather than left + // to this paragraph. + objectNavTargetExclusivity, } from '@objectstack/spec/ui'; import { BaseSchema, specFieldsExcept } from './base.zod.js'; import { handlerKeyRefusal } from './tombstone.zod.js'; @@ -105,12 +112,12 @@ const NavigationItemObject = z.object({ viewName: z.string().optional().describe('Target view name (type: object) — named list view e.g. calendar, pipeline'), recordId: z.string().optional().describe('Target record id (type: object) — opens a single record. Supports template variables {current_user_id}, {current_org_id}.'), recordMode: z.enum(['view', 'edit']).optional().describe('Record opening mode when recordId is set (default: view)'), - filters: z.record(z.string(), z.string()).optional().describe('URL filter conditions (type: object) — targets the /:objectName/data bare surface via filter[]= params instead of a saved view. Values support {current_user_id}/{current_org_id}. Precedence: recordId → filters → viewName.'), + filters: z.record(z.string(), z.string()).optional().describe('URL filter conditions (type: object) — targets the /:objectName/data bare surface via filter[]= params instead of a saved view. Values support {current_user_id}/{current_org_id}. Mutually exclusive with recordId/viewName.'), // Declared here for the same reason `requiresObject` / `actionDef` are: this // schema STRIPS unknown keys, so an entry deep-linking into an action would // have validated clean through `objectui validate` with the deep link thrown // away (the objectstack#4115 failure class). Spec: `ObjectNavItemSchema.runAction`. - runAction: z.string().optional().describe('Auto-run deep link (type: object) — name of an action on the target object that the list surface runs once on arrival. Ignored when recordId wins precedence (that resolves to a record page, not the list). Encoded as the reserved ?runAction= search param.'), + runAction: z.string().optional().describe('Auto-run deep link (type: object) — name of an action on the target object that the list surface runs once on arrival. Not combinable with recordId (a record detail page has no list toolbar to auto-run); composes with viewName or filters. Encoded as the reserved ?runAction= search param.'), dashboardName: z.string().optional().describe('Target dashboard name (type: dashboard)'), pageName: z.string().optional().describe('Target page name (type: page)'), reportName: z.string().optional().describe('Target report name (type: report)'), @@ -162,6 +169,24 @@ const NavigationItemObject = z.object({ }); } } + + // The spec's OWN target-exclusivity rule, CHAINED rather than restated + // (objectui#8563). This schema is hand-written — it is not `.shape`-derived — + // so no other mechanism carries the spec's checks across, and a local copy of + // the rule body would drift the day the spec's own moves. `@objectstack/spec` + // mounts this same function on the `type: 'object'` branch of ITS + // `NavigationItemSchema`, so an item the spec door refuses is refused here too + // instead of passing here and failing at publish. + // + // ⚠️ The rule is deliberately NOT pairwise-exclusive over the target fields, + // and chaining is what keeps that from being re-derived wrongly from prose: + // `recordId` + `viewName` is TOLERATED, and `runAction` is refused with + // `recordId` ONLY — it composes with `viewName` or `filters`. Both asymmetries, + // and the identity of the chained function, are pinned in + // `../__tests__/nav-target-exclusivity-8563.test.ts`. + if (item.type === 'object') { + objectNavTargetExclusivity(item, ctx); + } }); export const NavigationItemSchema: z.ZodType = z.lazy(() => NavigationItemObject); diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 1f9e1bfcc6..677d25bcc6 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -2810,7 +2810,7 @@ importers: packages/types: dependencies: '@objectstack/spec': - specifier: ^17.3.0 + specifier: ^17.4.0 version: 17.4.0(ai@7.0.65(zod@4.4.3)) zod: specifier: ^4.4.3