diff --git a/.changeset/action-param-strict-unknown-keys.md b/.changeset/action-param-strict-unknown-keys.md new file mode 100644 index 0000000000..0fa7b1337a --- /dev/null +++ b/.changeset/action-param-strict-unknown-keys.md @@ -0,0 +1,40 @@ +--- +"@objectstack/spec": minor +--- + +feat(spec): reject unknown keys on an action param instead of stripping them (#3405) + +`ActionParamSchema` was zod-default `.strip`: any key it does not declare was +**discarded silently** and the param went on parsing. That is the mechanism +behind the `reference` bug — an author wrote a correct, clearly intended +`reference: 'sys_user'`, the key was eaten, and the param dialog rendered a text +box asking a human to paste a UUID. Adding `reference` fixed that one key; the +mechanism that swallowed it stayed, so the next mis-spelled key would fail the +same way, with the same zero feedback (ADR-0078 no-silently-inert-metadata, +ADR-0049 enforce-or-remove). + +An action param is now `.strict()`. An undeclared key is a parse error naming the +offending key, and — when the key is a recognisable spelling of a declared one — +the canonical key to use instead: + +``` +Unrecognized key(s) on this action param: `reference_to`. Until #3405 these were +dropped silently — the param still parsed, so a mis-spelled config shipped as a +control that quietly ignored it. Did you mean `reference_to` → `reference`? +``` + +**Migration.** A param that previously carried an extra key now fails to parse. +The fix is to correct or remove that key; the error names it. Common mappings — +case/underscore slips are matched automatically, these are the ones that need a +different word: + +| Wrote | Use | +|---|---| +| `reference_to` / `referenceTo` / `targetObject` | `reference` | +| `visibleWhen` / `visibleOn` / `visibility` | `visible` | +| `description` / `help` | `helpText` | +| `default` | `defaultValue` | + +Declared keys are unchanged: `name`, `field`, `objectOverride`, `label`, `type`, +`required`, `options`, `placeholder`, `helpText`, `defaultValue`, `multiple`, +`accept`, `maxSize`, `reference`, `defaultFromRow`, `visible`, `requiresFeature`. diff --git a/packages/spec/src/ui/action.test.ts b/packages/spec/src/ui/action.test.ts index 329ac3d502..f8a5951fe7 100644 --- a/packages/spec/src/ui/action.test.ts +++ b/packages/spec/src/ui/action.test.ts @@ -108,6 +108,73 @@ describe('ActionParamSchema', () => { expect(ActionParamSchema.parse({ name: 'note', type: 'textarea' as const }).reference).toBeUndefined(); }); }); + + // #3405 part 3 — the root cause behind the `reference` bug was not the missing + // key, it was that an undeclared key was dropped *silently*: the param went on + // parsing and shipped a control that ignored the author's config. Strict mode + // turns that class of typo into a loud, fixable parse error + // (ADR-0078 no-silently-inert-metadata, ADR-0049 enforce-or-remove). + describe('unknown keys are rejected, not stripped (#3405 part 3)', () => { + const unknownKeyIssue = (param: Record) => { + const result = ActionParamSchema.safeParse(param); + expect(result.success).toBe(false); + return result.error!.issues.find((i) => i.code === 'unrecognized_keys'); + }; + + it('rejects an undeclared key instead of silently dropping it', () => { + const issue = unknownKeyIssue({ name: 'p', type: 'text', notAKey: 'x' }); + expect(issue).toBeDefined(); + expect(issue!.message).toContain('`notAKey`'); + }); + + it('points a snake_case mis-spelling at the declared camelCase key', () => { + // FieldSchema-adjacent metadata is snake_case, so this is the likely slip. + expect(unknownKeyIssue({ name: 'p', type: 'text', help_text: 'hi' })!.message) + .toContain('`help_text` → `helpText`'); + expect(unknownKeyIssue({ name: 'p', type: 'text', default_value: 1 })!.message) + .toContain('`default_value` → `defaultValue`'); + }); + + it('points the runtime lookup-target spellings at `reference` (the #3405 slip)', () => { + for (const key of ['reference_to', 'referenceTo', 'targetObject']) { + expect(unknownKeyIssue({ name: 'p', type: 'lookup', reference: 'sys_user', [key]: 'sys_user' })!.message) + .toContain(`\`${key}\` → \`reference\``); + } + }); + + it('points `visibleWhen` at `visible` so a capability gate cannot go inert', () => { + // ADR-0089 made `visibleWhen` canonical on view/page schemas; borrowing it + // here used to strip the gate and render the param unconditionally. + expect(unknownKeyIssue({ name: 'p', type: 'text', visibleWhen: 'features.x == true' })!.message) + .toContain('`visibleWhen` → `visible`'); + }); + + it('still reports an unrecognisable key without a bogus suggestion', () => { + const message = unknownKeyIssue({ name: 'p', type: 'text', wibble: 1 })!.message; + expect(message).toContain('`wibble`'); + expect(message).not.toContain('Did you mean'); + }); + + it('accepts every key the schema declares (guards ACTION_PARAM_KEYS drift)', () => { + // If a declared key were missing from the suggestion list, or a listed key + // were removed from the schema, one of these probes would be rejected. + const probes: Record = { + name: 'p', field: 'inspector', objectOverride: 'sys_member', label: 'P', + type: 'lookup', required: true, options: [{ label: 'A', value: 'a' }], + placeholder: 'ph', helpText: 'help', defaultValue: 'd', multiple: true, + accept: ['image/*'], maxSize: 1024, reference: 'sys_user', + defaultFromRow: true, visible: 'features.phoneNumber == true', + requiresFeature: 'phoneNumber', + }; + for (const [key, value] of Object.entries(probes)) { + const result = ActionParamSchema.safeParse({ name: 'p', [key]: value }); + const unknown = result.success + ? undefined + : result.error.issues.find((i) => i.code === 'unrecognized_keys'); + expect(unknown, `\`${key}\` should be a declared ActionParam key`).toBeUndefined(); + } + }); + }); }); // #2874 P1 — declarative `requiresFeature` sugar, lowered at parse time into diff --git a/packages/spec/src/ui/action.zod.ts b/packages/spec/src/ui/action.zod.ts index 6b2b298398..701ebeafc6 100644 --- a/packages/spec/src/ui/action.zod.ts +++ b/packages/spec/src/ui/action.zod.ts @@ -9,6 +9,7 @@ import { HookBodySchema } from '../data/hook-body.zod'; // Imported file-directly (not via the kernel barrel): the module is // deliberately import-free, so this cannot introduce a cycle. import { PUBLIC_AUTH_FEATURE_NAMES, lowerRequiresFeature } from '../kernel/public-auth-features'; +import { findClosestMatches } from '../shared/suggestions.zod'; /** * Action Parameter Schema @@ -45,6 +46,86 @@ import { PUBLIC_AUTH_FEATURE_NAMES, lowerRequiresFeature } from '../kernel/publi * to the field name and is used as the request-body key). */ import { lazySchema } from '../shared/lazy-schema'; + +/** + * Keys `ActionParamSchema` declares. + * + * Kept beside the schema rather than derived from `.shape`: the schema body is + * allocated lazily (see `lazySchema`), and the error map below has to name a + * canonical key *while* that first parse is still in flight. `action.zod.test.ts` + * asserts every entry here is really accepted, so the list cannot rot silently. + */ +const ACTION_PARAM_KEYS = [ + 'name', 'field', 'objectOverride', 'label', 'type', 'required', 'options', + 'placeholder', 'helpText', 'defaultValue', 'multiple', 'accept', 'maxSize', + 'reference', 'defaultFromRow', 'visible', 'requiresFeature', +] as const; + +/** + * Semantic near-misses — a different **word** for the same intent, usually + * borrowed from a neighbouring schema where that word is correct. Edit distance + * cannot reach these (`visibleWhen` → `visible` is 4 apart), so they are named + * explicitly; plain case/underscore slips (`help_text` → `helpText`) are left to + * {@link findClosestMatches}. Mirrors the `FIELD_TYPE_ALIASES` pattern in + * `shared/suggestions.zod.ts`. + * + * Keys are normalised by {@link aliasProbe} — lowercase, separators removed. + */ +const ACTION_PARAM_KEY_ALIASES: Readonly> = { + // The objectql/runtime field shape spells a lookup target `reference_to`, and + // objectui's resolved param calls it `referenceTo`. Dropping either is the + // exact #3405 failure: a targetless picker degrades to a raw-UUID text box. + referenceto: 'reference', + referenceobject: 'reference', + referencedobject: 'reference', + targetobject: 'reference', + // ADR-0089 made `visibleWhen` the canonical predicate on view/page schemas. + // An author who learned it there would silently lose a param's capability + // gate here — the param would render unconditionally. + visiblewhen: 'visible', + visibleon: 'visible', + visibility: 'visible', + description: 'helpText', + help: 'helpText', + default: 'defaultValue', +}; + +/** `reference_to` / `referenceTo` / `Reference-To` all collapse onto one probe. */ +const aliasProbe = (key: string): string => key.toLowerCase().replace(/[_\-\s]/g, ''); + +/** + * Custom zod `error` for the `.strict()` {@link ActionParamSchema} (#3405 part 3). + * + * Before this, the schema was zod-default `.strip`: a key it does not declare was + * **silently discarded**, and the param went on parsing. That is how a correctly + * intended `reference: 'sys_user'` became a text box asking a human to paste a + * UUID, with no error anywhere — the config was eaten and the UI lied about why + * (ADR-0078 no-silently-inert-metadata, ADR-0049 enforce-or-remove). + * + * Strict alone would only say "unrecognized key". This map makes the rejection + * *fixable*: it names the offending key(s) and, when one is a recognisable + * spelling of a declared key, points at the canonical one. + */ +const actionParamUnknownKeyError: z.core.$ZodErrorMap = (issue) => { + if (issue.code !== 'unrecognized_keys') return undefined; + const keys = (issue as { keys?: readonly string[] }).keys ?? []; + const suggestions = keys.flatMap((key) => { + // Length-relative bound, matching `suggestKey` in `data/object.zod.ts`: a + // flat distance of 3 is noise on a short key (`wibble` → `visible`). + const maxDistance = Math.max(2, Math.floor(key.length / 3)); + const canonical = + ACTION_PARAM_KEY_ALIASES[aliasProbe(key)] ?? + findClosestMatches(key, ACTION_PARAM_KEYS, maxDistance, 1)[0]; + return canonical && canonical !== key ? [`\`${key}\` → \`${canonical}\``] : []; + }); + const base = + `Unrecognized key(s) on this action param: ${keys.map((k) => `\`${k}\``).join(', ')}. ` + + `Until #3405 these were dropped silently — the param still parsed, so a mis-spelled ` + + `config shipped as a control that quietly ignored it.`; + return suggestions.length ? `${base} Did you mean ${suggestions.join(', ')}?` : base; +}; + + export const ActionParamSchema = lazySchema(() => z.object({ /** Request-body key. Defaults to `field` when `field` is set. */ name: z.string().optional(), @@ -123,7 +204,7 @@ export const ActionParamSchema = lazySchema(() => z.object({ * enum-checked and the gate/registry stay in lockstep. */ requiresFeature: z.enum(PUBLIC_AUTH_FEATURE_NAMES).optional().describe('Public auth feature flag gating this param; lowered into `visible` at parse time.'), -}).refine( +}, { error: actionParamUnknownKeyError }).strict().refine( (p) => Boolean(p.name) || Boolean(p.field), { message: 'ActionParam requires either "name" or "field"' }, ).refine( diff --git a/skills/objectstack-data/references/_index.md b/skills/objectstack-data/references/_index.md index 0c5e0c423f..e1c19ed139 100644 --- a/skills/objectstack-data/references/_index.md +++ b/skills/objectstack-data/references/_index.md @@ -27,6 +27,7 @@ from `node_modules` — there is no local copy in the skill bundle. - `node_modules/@objectstack/spec/src/shared/http.zod.ts` — Shared HTTP Schemas - `node_modules/@objectstack/spec/src/shared/identifiers.zod.ts` — System Identifier Schema - `node_modules/@objectstack/spec/src/shared/protection.zod.ts` — Package-level metadata protection (ADR-0010 §3.7 — Phase 4.3) +- `node_modules/@objectstack/spec/src/shared/suggestions.zod.ts` — "Did you mean?" Suggestion Utilities - `node_modules/@objectstack/spec/src/ui/action.zod.ts` — Action Parameter Schema - `node_modules/@objectstack/spec/src/ui/i18n.zod.ts` — I18n Object Schema - `node_modules/@objectstack/spec/src/ui/responsive.zod.ts` — Breakpoint Name Enum diff --git a/skills/objectstack-platform/references/_index.md b/skills/objectstack-platform/references/_index.md index b39cd4171f..8eaee01b08 100644 --- a/skills/objectstack-platform/references/_index.md +++ b/skills/objectstack-platform/references/_index.md @@ -32,6 +32,7 @@ from `node_modules` — there is no local copy in the skill bundle. - `node_modules/@objectstack/spec/src/shared/expression.zod.ts` — Expression Protocol - `node_modules/@objectstack/spec/src/shared/identifiers.zod.ts` — System Identifier Schema - `node_modules/@objectstack/spec/src/shared/protection.zod.ts` — Package-level metadata protection (ADR-0010 §3.7 — Phase 4.3) +- `node_modules/@objectstack/spec/src/shared/suggestions.zod.ts` — "Did you mean?" Suggestion Utilities - `node_modules/@objectstack/spec/src/system/tenant.zod.ts` — Tenant Schema (Multi-Tenant Architecture) - `node_modules/@objectstack/spec/src/ui/action.zod.ts` — Action Parameter Schema - `node_modules/@objectstack/spec/src/ui/app.zod.ts` — Base Navigation Item Schema diff --git a/skills/objectstack-ui/references/_index.md b/skills/objectstack-ui/references/_index.md index 26456bf8cf..0a1e014beb 100644 --- a/skills/objectstack-ui/references/_index.md +++ b/skills/objectstack-ui/references/_index.md @@ -34,6 +34,7 @@ from `node_modules` — there is no local copy in the skill bundle. - `node_modules/@objectstack/spec/src/shared/http.zod.ts` — Shared HTTP Schemas - `node_modules/@objectstack/spec/src/shared/identifiers.zod.ts` — System Identifier Schema - `node_modules/@objectstack/spec/src/shared/protection.zod.ts` — Package-level metadata protection (ADR-0010 §3.7 — Phase 4.3) +- `node_modules/@objectstack/spec/src/shared/suggestions.zod.ts` — "Did you mean?" Suggestion Utilities - `node_modules/@objectstack/spec/src/ui/i18n.zod.ts` — I18n Object Schema - `node_modules/@objectstack/spec/src/ui/responsive.zod.ts` — Breakpoint Name Enum - `node_modules/@objectstack/spec/src/ui/sharing.zod.ts` — Sharing & Embedding Protocol