From f07277b1ec80d7224427e005805298532e777c46 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 28 Jul 2026 01:36:29 +0000 Subject: [PATCH 1/3] feat(spec): reject unknown keys on an action param instead of stripping them (#3405) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes part 3 of #3405 — the item deferred out of #3406 as "evaluate separately". Parts 1 and 2 gave an inline record-picker param a `reference` key and made a targetless one a parse error. That fixed the symptom. The mechanism that caused it stayed: `ActionParamSchema` was zod-default `.strip`, so any key it does not declare was discarded silently and the param went on parsing. An author wrote a correct, clearly intended `reference: 'sys_user'`, the key was eaten, and the dialog rendered a text box asking a human to paste a UUID — no error anywhere. The next mis-spelled key would have failed the same way, just as quietly. An action param is now `.strict()`, with an error map that makes the rejection fixable rather than merely loud: - Case/underscore slips (`help_text` → `helpText`, `default_value` → `defaultValue`) resolve through the shared `findClosestMatches`, bounded by the same length-relative distance `suggestKey` uses in `data/object.zod.ts` — a flat distance of 3 suggests `visible` for `wibble`. - Semantic near-misses edit distance cannot reach are named explicitly, in the `FIELD_TYPE_ALIASES` style: `reference_to` / `referenceTo` / `targetObject` → `reference` (the runtime field shape spells it the first way, objectui's resolved param the second), and `visibleWhen` / `visibleOn` / `visibility` → `visible`. That last one is why this matters beyond typos: ADR-0089 made `visibleWhen` canonical on view/page schemas, so borrowing it here used to strip a param's capability gate and render it unconditionally. Follows ADR-0078 (no-silently-inert-metadata) and ADR-0049 (enforce-or-remove), and matches the precedent set by ADR-0089 D3a for the view/page schemas. Verification: spec 258 files / 6716 tests pass; `tsc --noEmit` clean; app-showcase, app-crm and app-todo all `validate` clean — no existing metadata in the repo carried an undeclared param key. Planting `visibleWhen` on showcase's inline picker param reproduces the new error with the `visibleWhen` → `visible` prescription, and removing it validates again. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01LkvDs4y4gveyB5NJZKxaZt --- .../action-param-strict-unknown-keys.md | 40 +++++++++ packages/spec/src/ui/action.test.ts | 67 +++++++++++++++ packages/spec/src/ui/action.zod.ts | 82 ++++++++++++++++++- 3 files changed, 188 insertions(+), 1 deletion(-) create mode 100644 .changeset/action-param-strict-unknown-keys.md 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..05bc87d360 100644 --- a/packages/spec/src/ui/action.zod.ts +++ b/packages/spec/src/ui/action.zod.ts @@ -9,6 +9,85 @@ 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'; + +/** + * 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. + */ +export 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; +}; /** * Action Parameter Schema @@ -45,6 +124,7 @@ 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'; + export const ActionParamSchema = lazySchema(() => z.object({ /** Request-body key. Defaults to `field` when `field` is set. */ name: z.string().optional(), @@ -123,7 +203,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( From 95b154abd3a3f21aba9bd410331a86cf46f2aaa6 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 28 Jul 2026 01:41:24 +0000 Subject: [PATCH 2/3] fix(spec): keep the schema's own JSDoc first in action.zod.ts (#3405) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `scripts/build-docs.ts` `getFileDescription()` takes the FIRST `/** */` block in a module, verbatim, as the description of its generated reference page. Placing the new `ACTION_PARAM_KEYS` / error-map helpers above the "Action Parameter Schema" JSDoc therefore replaced the public authoring guide on `content/docs/references/ui/action.mdx` with an internal note about why a key list is kept beside the schema — which is what `check:docs` caught. Moved the helpers back below that JSDoc (after the `lazySchema` import, where they were originally). The generated doc is byte-identical to main again: `check:docs` reports 250 generated files in sync. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01LkvDs4y4gveyB5NJZKxaZt --- packages/spec/src/ui/action.zod.ts | 71 +++++++++++++++--------------- 1 file changed, 36 insertions(+), 35 deletions(-) diff --git a/packages/spec/src/ui/action.zod.ts b/packages/spec/src/ui/action.zod.ts index 05bc87d360..4eac7f9aaa 100644 --- a/packages/spec/src/ui/action.zod.ts +++ b/packages/spec/src/ui/action.zod.ts @@ -11,6 +11,42 @@ import { HookBodySchema } from '../data/hook-body.zod'; import { PUBLIC_AUTH_FEATURE_NAMES, lowerRequiresFeature } from '../kernel/public-auth-features'; import { findClosestMatches } from '../shared/suggestions.zod'; +/** + * Action Parameter Schema + * + * Defines inputs required before executing an action. + * + * Two declaration modes: + * + * 1. **Field-backed** (preferred) — reference an existing object field; the + * runtime resolves the field's label (i18n), type, validation rules, + * options, placeholder, help text, and widget mapping from object + * metadata. Cross-object references use `objectOverride`. + * + * ```ts + * params: [ + * { field: 'email' }, // same object + * { field: 'role', objectOverride: 'sys_member' }, // different object + * ] + * ``` + * + * 2. **Inline** (legacy / bespoke) — declare `name`, `label`, `type` etc. + * inline when no matching object field exists. Inline values may also be + * used alongside `field` to override individual properties. A `lookup` / + * `master_detail` param declared this way MUST name its target object via + * `reference` — there is no field to inherit it from: + * + * ```ts + * params: [ + * { name: 'inspector', label: 'Inspector', type: 'lookup', reference: 'sys_user' }, + * ] + * ``` + * + * `name` is required unless `field` is provided (in which case it defaults + * to the field name and is used as the request-body key). + */ +import { lazySchema } from '../shared/lazy-schema'; + /** * Keys `ActionParamSchema` declares. * @@ -89,41 +125,6 @@ export const actionParamUnknownKeyError: z.core.$ZodErrorMap = (issue) => { return suggestions.length ? `${base} Did you mean ${suggestions.join(', ')}?` : base; }; -/** - * Action Parameter Schema - * - * Defines inputs required before executing an action. - * - * Two declaration modes: - * - * 1. **Field-backed** (preferred) — reference an existing object field; the - * runtime resolves the field's label (i18n), type, validation rules, - * options, placeholder, help text, and widget mapping from object - * metadata. Cross-object references use `objectOverride`. - * - * ```ts - * params: [ - * { field: 'email' }, // same object - * { field: 'role', objectOverride: 'sys_member' }, // different object - * ] - * ``` - * - * 2. **Inline** (legacy / bespoke) — declare `name`, `label`, `type` etc. - * inline when no matching object field exists. Inline values may also be - * used alongside `field` to override individual properties. A `lookup` / - * `master_detail` param declared this way MUST name its target object via - * `reference` — there is no field to inherit it from: - * - * ```ts - * params: [ - * { name: 'inspector', label: 'Inspector', type: 'lookup', reference: 'sys_user' }, - * ] - * ``` - * - * `name` is required unless `field` is provided (in which case it defaults - * to the field name and is used as the request-body key). - */ -import { lazySchema } from '../shared/lazy-schema'; export const ActionParamSchema = lazySchema(() => z.object({ /** Request-body key. Defaults to `field` when `field` is set. */ From 84c8ca0e866d9b608520d943c2a04ffd2d719c51 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 28 Jul 2026 01:49:21 +0000 Subject: [PATCH 3/3] fix(spec): regenerate skill references, keep the error map internal (#3405) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two more generated-artifact gates behind `check:docs`, both tripped by the same commit: - `check:skill-refs` — importing `shared/suggestions.zod.ts` from `ui/action.zod.ts` pulls it into the transitive reference set of the objectstack-data / -ui / -platform skills. Regenerated via `gen:skill-refs`; the diff is the one expected line per skill, and `action.zod.ts` still resolves to "Action Parameter Schema", confirming the JSDoc-order fix held. - `check:api-surface` — `actionParamUnknownKeyError` was exported, which added it to the package's public API. It has no caller outside its own module, so the export was unnecessary: unlike `strictVisibilityError`, which is shared across the view/page schemas, this map is wired into exactly one schema. Made it module-private; the public API surface is now unchanged by this PR. All ten `check:*` gates in packages/spec pass locally, alongside 258 files / 6716 tests and a clean `tsc --noEmit`. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01LkvDs4y4gveyB5NJZKxaZt --- packages/spec/src/ui/action.zod.ts | 2 +- skills/objectstack-data/references/_index.md | 1 + skills/objectstack-platform/references/_index.md | 1 + skills/objectstack-ui/references/_index.md | 1 + 4 files changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/spec/src/ui/action.zod.ts b/packages/spec/src/ui/action.zod.ts index 4eac7f9aaa..701ebeafc6 100644 --- a/packages/spec/src/ui/action.zod.ts +++ b/packages/spec/src/ui/action.zod.ts @@ -106,7 +106,7 @@ const aliasProbe = (key: string): string => key.toLowerCase().replace(/[_\-\s]/g * *fixable*: it names the offending key(s) and, when one is a recognisable * spelling of a declared key, points at the canonical one. */ -export const actionParamUnknownKeyError: z.core.$ZodErrorMap = (issue) => { +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) => { 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