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
49 changes: 49 additions & 0 deletions .changeset/action-param-default-value-contract.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
---
"@objectstack/spec": minor
---

fix(spec): an action param's `defaultValue` is validated against the param's own value contract (#6970)

`ActionParamSchema.defaultValue` was `z.unknown().optional()`, so a default that
could never satisfy its own param was accepted at authoring time with no warning,
prefilled into the dialog control, and refused only at submit — on a field the
user never touched, by a message that named the param but not the author's
default as the cause.

The default is now checked at parse time through the **same** `valueSchemaFor`
the dispatcher already runs at submit (ADR-0104 D2, `validateActionParams`) —
one rule set, two moments, no second vocabulary. `datetime` was the loudest
instance (`'2026-08-10T15:00'`, a wall clock `datetime-local` happily displays
and `InstantValueSchema` refuses), but the hole was every type: `number` +
`'abc'`, `select` + a non-member, a `multiple` param + a scalar.

The rejection names the param, its type, the offending literal, and why it
matters:

```
Action param "start" (datetime): the default "2026-08-10T15:00" cannot satisfy
this param's own value contract — expected an ISO-8601 instant with explicit
zone (e.g. 2026-03-15T14:30:00.000Z). The dialog would PREFILL this value and
the submit would then be refused with that same message (ADR-0104 D2), for a
field the user never touched …
```

**Acceptance tightening — what is NOT judged.** The gate only answers what the
declaration itself can answer, because an authoring gate that guesses rejects
valid metadata. A param with no `type` of its own keeps an open value shape (the
same default `validateActionParams` applies to an unresolvable type); a
field-backed param that inherits its arity or its option set is not held to
either; and `null` / `''` defaults are skipped exactly as the dispatcher's own
presence check skips them.

**Stock compatibility.** Already-stored action metadata carrying a nonconforming
default keeps loading and keeps working: the read path (`DatabaseLoader.rowToData`)
replays the ADR-0087 conversion chain but runs no Zod validation, and
`MetadataManager.validate` is deliberately a structural check only. Authoritative
spec validation lives on the WRITE path (`protocol.saveMetaItem`) and is surfaced
on reads as the advisory `_diagnostics` envelope — which now reports the
nonconforming default instead of staying silent about it. So this is loud at
authoring, non-fatal at rest, and no conversion is owed: there is no mechanical
rewrite for "the author meant some other instant", and inventing one would pick a
timezone the metadata never declared (the ambiguity #5061 refused to resolve
consumer-side).
1 change: 1 addition & 0 deletions packages/spec/api-surface/ui.json
Original file line numberDiff line numberDiff line change
Expand Up@@ -389,6 +389,7 @@
"diagnoseViewMetadata (function)",
"expandViewContainer (function)",
"expandViewContainerWithDiagnostics (function)",
"isActionParamValuePresent (function)",
"isAggregatedViewContainer (function)",
"isRecordContextBlockType (function)",
"normalizeFilterOperator (function)",
Expand Down
1 change: 1 addition & 0 deletions packages/spec/export-origins/ui.json
Original file line numberDiff line numberDiff line change
Expand Up@@ -389,6 +389,7 @@
"diagnoseViewMetadata": "src/ui/view.zod.ts#diagnoseViewMetadata (function)",
"expandViewContainer": "src/ui/view.zod.ts#expandViewContainer (function)",
"expandViewContainerWithDiagnostics": "src/ui/view.zod.ts#expandViewContainerWithDiagnostics (function)",
"isActionParamValuePresent": "src/ui/action-params.zod.ts#isActionParamValuePresent (function)",
"isAggregatedViewContainer": "src/ui/view.zod.ts#isAggregatedViewContainer (function)",
"isRecordContextBlockType": "src/ui/react-blocks.ts#isRecordContextBlockType (function)",
"normalizeFilterOperator": "src/ui/view.zod.ts#normalizeFilterOperator (function)",
Expand Down
220 changes: 220 additions & 0 deletions packages/spec/src/ui/action-param-default-value.test.ts
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,220 @@
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.

/**
* #6970 — an action param's `defaultValue` must satisfy the param's OWN value
* contract at AUTHORING time, checked through the same `valueSchemaFor` the
* dispatcher runs at submit (ADR-0104 D2).
*
* Before this, `defaultValue` was `z.unknown().optional()`: a default that
* could never satisfy its own param parsed clean, prefilled the control, and
* 400'd at submit on a field the user never touched.
*/

import { describe, it, expect } from 'vitest';

import { ActionParamSchema } from './action.zod';
import { validateActionParams } from './action-params.zod';
import { getMetadataTypeSchema } from '../kernel/metadata-type-schemas';

/** Parse a param and return its first `defaultValue` issue, or `null`. */
function defaultValueIssue(param: Record<string, unknown>) {
const r = ActionParamSchema.safeParse(param);
if (r.success) return null;
return r.error.issues.find((i) => i.path.join('.') === 'defaultValue') ?? null;
}

/** What the ADR-0104 D2 dispatcher does with this default AS A SUBMITTED VALUE. */
function submitIssue(param: Record<string, unknown>) {
const issues = validateActionParams(
[{
name: param.name as string,
type: param.type as string | undefined,
multiple: param.multiple as boolean | undefined,
options: param.options as never,
}],
{ [param.name as string]: param.defaultValue },
);
return issues[0] ?? null;
}

/**
* The cases the issue names, plus the deliberate NON-rejections. `accepted`
* is the verdict for BOTH moments — that equivalence is the point (see the
* parity test at the bottom), so the table carries one column, not two.
*/
const CASES: Array<{ label: string; param: Record<string, unknown>; accepted: boolean }> = [
// ── The hole, per type ────────────────────────────────────────────────────
{
label: "datetime + a wall-clock literal (the issue's example)",
param: { name: 'start', type: 'datetime', defaultValue: '2026-08-10T15:00' },
accepted: false,
},
{ label: "number + 'abc'", param: { name: 'qty', type: 'number', defaultValue: 'abc' }, accepted: false },
{
label: 'select + a value not in its own options',
param: {
name: 'tier',
type: 'select',
options: [{ label: 'Gold', value: 'gold' }, { label: 'Silver', value: 'silver' }],
defaultValue: 'platinum',
},
accepted: false,
},
{
label: 'multiple: true + a scalar default',
param: { name: 'owners', type: 'user', multiple: true, defaultValue: 'usr_1' },
accepted: false,
},
{
label: 'date + a full instant (the mirror of the datetime case)',
param: { name: 'due', type: 'date', defaultValue: '2026-08-10T15:00:00.000Z' },
accepted: false,
},
{ label: 'boolean + a string', param: { name: 'notify', type: 'boolean', defaultValue: 'yes' }, accepted: false },
{
label: 'lookup + an embedded record object instead of an id',
param: { name: 'owner', type: 'lookup', reference: 'sys_user', defaultValue: { id: 'usr_1', name: 'Ada' } },
accepted: false,
},

// ── Valid defaults of each type: untouched ────────────────────────────────
{
label: 'VALID datetime (ISO instant with zone)',
param: { name: 'start', type: 'datetime', defaultValue: '2026-08-10T15:00:00.000Z' },
accepted: true,
},
{ label: 'VALID number', param: { name: 'qty', type: 'number', defaultValue: 7 }, accepted: true },
{
label: 'VALID select member',
param: { name: 'tier', type: 'select', options: [{ label: 'Gold', value: 'gold' }], defaultValue: 'gold' },
accepted: true,
},
{
label: 'VALID multiple array',
param: { name: 'owners', type: 'user', multiple: true, defaultValue: ['usr_1'] },
accepted: true,
},
{ label: 'VALID date', param: { name: 'due', type: 'date', defaultValue: '2026-08-10' }, accepted: true },
{ label: 'VALID boolean', param: { name: 'notify', type: 'boolean', defaultValue: true }, accepted: true },
{
label: 'json — an explicitly OPEN value contract, so any default rides',
param: { name: 'blob', type: 'json', defaultValue: { anything: ['at', 'all'] } },
accepted: true,
},
{
label: 'no `type` — the value shape is unresolvable, so it stays open',
param: { name: 'loose', defaultValue: 'whatever' },
accepted: true,
},

// ── Presence parity: the dispatcher treats these as ABSENT ────────────────
{
label: "empty-string default on a datetime — ABSENT at submit, so not judged here either",
param: { name: 'start', type: 'datetime', defaultValue: '' },
accepted: true,
},
{
label: 'null default on a number — ABSENT at submit',
param: { name: 'qty', type: 'number', defaultValue: null },
accepted: true,
},
];

describe('#6970 ActionParamSchema.defaultValue — authored defaults meet the param value contract', () => {
for (const { label, param, accepted } of CASES) {
it(`${accepted ? 'accepts' : 'rejects'}: ${label}`, () => {
const issue = defaultValueIssue(param);
if (accepted) {
expect(issue).toBeNull();
return;
}
// Rejection pin: this is a pure Zod parse (no error envelope), so the
// assertion set is issue PATH + message shape — never a bare
// `success === false`, which cannot tell this rejection from the
// schema refusing the param for some unrelated reason.
expect(issue).not.toBeNull();
expect(issue!.path).toEqual(['defaultValue']);
expect(issue!.code).toBe('custom');
// Names the param, its type, and the offending literal — the three
// things the submit-time 400 could not say.
expect(issue!.message).toContain(`"${param.name as string}"`);
expect(issue!.message).toContain(`(${param.type as string})`);
expect(issue!.message).toContain(JSON.stringify(param.defaultValue));
});
}

it("names the author's default as the cause, not just the param", () => {
const issue = defaultValueIssue({ name: 'start', type: 'datetime', defaultValue: '2026-08-10T15:00' })!;
// The underlying reason is carried verbatim from the shared value contract,
// so authoring and submit read identically.
expect(issue.message).toContain('expected an ISO-8601 instant with explicit zone');
expect(issue.message).toContain('cannot satisfy this param');
expect(issue.message).toContain('PREFILL');
});

it('reports through the real authoring door with a full params path', () => {
const r = getMetadataTypeSchema('action')!.safeParse({
name: 'schedule_visit',
label: 'Schedule Visit',
type: 'script',
params: [
{ name: 'note', type: 'text' },
{ name: 'start', type: 'datetime', defaultValue: '2026-08-10T15:00' },
],
});
expect(r.success).toBe(false);
const paths = r.error!.issues.map((i) => i.path.join('.'));
expect(paths).toContain('params.1.defaultValue');
});

// ── The design's two deliberate non-rejections ────────────────────────────
it('does NOT judge arity it cannot know — a field-backed param inherits `multiple`', () => {
// `{ field: 'owners' }` inherits `multiple: true` from the referenced
// field, which is invisible at parse time. Rejecting this array would be
// the authoring gate guessing, and guessing wrong rejects valid metadata.
expect(defaultValueIssue({ field: 'owners', type: 'user', defaultValue: ['usr_1', 'usr_2'] })).toBeNull();
// The scalar spelling of the same inherited-arity param is equally legal.
expect(defaultValueIssue({ field: 'owners', type: 'user', defaultValue: 'usr_1' })).toBeNull();
});

it('still judges arity when the param STATES it, field-backed or not', () => {
// `multiple` declared → the declaration answers the question, so it binds.
expect(defaultValueIssue({ field: 'owners', type: 'user', multiple: true, defaultValue: 'usr_1' }))
.not.toBeNull();
// Inline (no `field`) → nothing to inherit from, so silence means scalar.
expect(defaultValueIssue({ name: 'owners', type: 'user', defaultValue: ['usr_1'] })).not.toBeNull();
});

it('does NOT judge option membership it cannot know — inherited option sets stay open', () => {
// No inline `options`: the set comes from the referenced field, so
// `valueSchemaFor` degrades to free-form and any string default rides.
expect(defaultValueIssue({ field: 'tier', type: 'select', defaultValue: 'gold' })).toBeNull();
});

it('leaves params WITHOUT a defaultValue completely untouched', () => {
for (const type of ['datetime', 'number', 'select', 'user', 'date', 'boolean']) {
expect(ActionParamSchema.safeParse({ name: 'x', type }).success).toBe(true);
}
});

/**
* The ruling's actual claim: `defaultValue` goes through the SAME
* `valueSchemaFor` machinery the dispatcher uses — no second rule set. This
* is the pin that would catch a future edit re-implementing the check by
* hand, which is how the two ends drift into two dialects.
*/
it('agrees with the dispatcher on every case — one rule set, two moments', () => {
for (const { label, param } of CASES) {
// Arity/membership the AUTHORING side deliberately cannot resolve are
// excluded: the dispatcher is fed the RESOLVED param, so for those rows
// the two sides are answering different questions by design.
if (param.field !== undefined) continue;
const authoringRejects = defaultValueIssue(param) !== null;
const submitRejects = submitIssue(param) !== null;
expect(
{ case: label, authoringRejects },
`authoring and submit must agree for: ${label}`,
).toEqual({ case: label, authoringRejects: submitRejects });
}
});
});
16 changes: 15 additions & 1 deletion packages/spec/src/ui/action-params.zod.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -114,10 +114,24 @@ const BUILTIN_PARAM_ORIGINS: ReadonlyMap<string, string> = new Map([
/** Fallback origin sentence for a built-in supplied via `opts.builtinKeys`. */
const GENERIC_BUILTIN_ORIGIN = 'the dispatcher supplies it.';

function isPresent(v: unknown): boolean {
/**
* Whether a value counts as PRESENT for action-param purposes — the one
* definition of "there is a value here to check".
*
* Exported because the AUTHORING gate on `ActionParamSchema.defaultValue`
* (#6970) must skip exactly what this dispatch path skips. An authored default
* of `null` or `''` never reaches {@link valueSchemaFor} at submit — it is
* treated as no value, and `required` decides the outcome — so a parse-time
* check that rejected `''` for not being an ISO instant would be a SECOND rule
* set, stricter than the contract it claims to enforce. Sharing the predicate
* makes that parity structural instead of remembered.
*/
export function isActionParamValuePresent(v: unknown): boolean {
return v !== undefined && v !== null && !(typeof v === 'string' && v.trim() === '');
}

const isPresent = isActionParamValuePresent;

/**
* The tail appended to an `unknown_field` message when the rejected key is one
* leading underscore away from a built-in — `''` when it is not (an ordinary
Expand Down
Loading
Loading