Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 40 additions & 0 deletions .changeset/action-param-strict-unknown-keys.md
Original file line number Diff line number Diff line change
@@ -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`.
67 changes: 67 additions & 0 deletions packages/spec/src/ui/action.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<string, unknown>) => {
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<string, unknown> = {
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
Expand Down
83 changes: 82 additions & 1 deletion packages/spec/src/ui/action.zod.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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<Record<string, string>> = {
// 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(),
Expand Down Expand Up @@ -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(
Expand Down
1 change: 1 addition & 0 deletions skills/objectstack-data/references/_index.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions skills/objectstack-platform/references/_index.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions skills/objectstack-ui/references/_index.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading