Skip to content

Commit 445a0c2

Browse files
os-zhuangclaude
andauthored
fix(spec): validate action param defaultValue against the param's own value contract (#6970) (#7126)
* fix(spec): validate action param defaultValue against the param's own value contract (#6970) * chore(spec): regenerate export/skill baselines for the new shared presence predicate (#6970) `action.zod.ts` now imports `field-value.zod` and `action-params.zod`, and `isActionParamValuePresent` is exported so the authoring gate and the dispatcher share one presence predicate. Regenerated AFTER `pnpm --filter @objectstack/spec build` — the api-surface/export-origins generators read the built dist, and running them against a stale dist silently drops unrelated entries. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PiRUoQkTSBBmpyXBY3cVn2 --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 2233a85 commit 445a0c2

9 files changed

Lines changed: 370 additions & 3 deletions

File tree

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
---
2+
"@objectstack/spec": minor
3+
---
4+
5+
fix(spec): an action param's `defaultValue` is validated against the param's own value contract (#6970)
6+
7+
`ActionParamSchema.defaultValue` was `z.unknown().optional()`, so a default that
8+
could never satisfy its own param was accepted at authoring time with no warning,
9+
prefilled into the dialog control, and refused only at submit — on a field the
10+
user never touched, by a message that named the param but not the author's
11+
default as the cause.
12+
13+
The default is now checked at parse time through the **same** `valueSchemaFor`
14+
the dispatcher already runs at submit (ADR-0104 D2, `validateActionParams`) —
15+
one rule set, two moments, no second vocabulary. `datetime` was the loudest
16+
instance (`'2026-08-10T15:00'`, a wall clock `datetime-local` happily displays
17+
and `InstantValueSchema` refuses), but the hole was every type: `number` +
18+
`'abc'`, `select` + a non-member, a `multiple` param + a scalar.
19+
20+
The rejection names the param, its type, the offending literal, and why it
21+
matters:
22+
23+
```
24+
Action param "start" (datetime): the default "2026-08-10T15:00" cannot satisfy
25+
this param's own value contract — expected an ISO-8601 instant with explicit
26+
zone (e.g. 2026-03-15T14:30:00.000Z). The dialog would PREFILL this value and
27+
the submit would then be refused with that same message (ADR-0104 D2), for a
28+
field the user never touched …
29+
```
30+
31+
**Acceptance tightening — what is NOT judged.** The gate only answers what the
32+
declaration itself can answer, because an authoring gate that guesses rejects
33+
valid metadata. A param with no `type` of its own keeps an open value shape (the
34+
same default `validateActionParams` applies to an unresolvable type); a
35+
field-backed param that inherits its arity or its option set is not held to
36+
either; and `null` / `''` defaults are skipped exactly as the dispatcher's own
37+
presence check skips them.
38+
39+
**Stock compatibility.** Already-stored action metadata carrying a nonconforming
40+
default keeps loading and keeps working: the read path (`DatabaseLoader.rowToData`)
41+
replays the ADR-0087 conversion chain but runs no Zod validation, and
42+
`MetadataManager.validate` is deliberately a structural check only. Authoritative
43+
spec validation lives on the WRITE path (`protocol.saveMetaItem`) and is surfaced
44+
on reads as the advisory `_diagnostics` envelope — which now reports the
45+
nonconforming default instead of staying silent about it. So this is loud at
46+
authoring, non-fatal at rest, and no conversion is owed: there is no mechanical
47+
rewrite for "the author meant some other instant", and inventing one would pick a
48+
timezone the metadata never declared (the ambiguity #5061 refused to resolve
49+
consumer-side).

packages/spec/api-surface/ui.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -389,6 +389,7 @@
389389
"diagnoseViewMetadata (function)",
390390
"expandViewContainer (function)",
391391
"expandViewContainerWithDiagnostics (function)",
392+
"isActionParamValuePresent (function)",
392393
"isAggregatedViewContainer (function)",
393394
"isRecordContextBlockType (function)",
394395
"normalizeFilterOperator (function)",

packages/spec/export-origins/ui.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -389,6 +389,7 @@
389389
"diagnoseViewMetadata": "src/ui/view.zod.ts#diagnoseViewMetadata (function)",
390390
"expandViewContainer": "src/ui/view.zod.ts#expandViewContainer (function)",
391391
"expandViewContainerWithDiagnostics": "src/ui/view.zod.ts#expandViewContainerWithDiagnostics (function)",
392+
"isActionParamValuePresent": "src/ui/action-params.zod.ts#isActionParamValuePresent (function)",
392393
"isAggregatedViewContainer": "src/ui/view.zod.ts#isAggregatedViewContainer (function)",
393394
"isRecordContextBlockType": "src/ui/react-blocks.ts#isRecordContextBlockType (function)",
394395
"normalizeFilterOperator": "src/ui/view.zod.ts#normalizeFilterOperator (function)",
Lines changed: 220 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,220 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* #6970 — an action param's `defaultValue` must satisfy the param's OWN value
5+
* contract at AUTHORING time, checked through the same `valueSchemaFor` the
6+
* dispatcher runs at submit (ADR-0104 D2).
7+
*
8+
* Before this, `defaultValue` was `z.unknown().optional()`: a default that
9+
* could never satisfy its own param parsed clean, prefilled the control, and
10+
* 400'd at submit on a field the user never touched.
11+
*/
12+
13+
import { describe, it, expect } from 'vitest';
14+
15+
import { ActionParamSchema } from './action.zod';
16+
import { validateActionParams } from './action-params.zod';
17+
import { getMetadataTypeSchema } from '../kernel/metadata-type-schemas';
18+
19+
/** Parse a param and return its first `defaultValue` issue, or `null`. */
20+
function defaultValueIssue(param: Record<string, unknown>) {
21+
const r = ActionParamSchema.safeParse(param);
22+
if (r.success) return null;
23+
return r.error.issues.find((i) => i.path.join('.') === 'defaultValue') ?? null;
24+
}
25+
26+
/** What the ADR-0104 D2 dispatcher does with this default AS A SUBMITTED VALUE. */
27+
function submitIssue(param: Record<string, unknown>) {
28+
const issues = validateActionParams(
29+
[{
30+
name: param.name as string,
31+
type: param.type as string | undefined,
32+
multiple: param.multiple as boolean | undefined,
33+
options: param.options as never,
34+
}],
35+
{ [param.name as string]: param.defaultValue },
36+
);
37+
return issues[0] ?? null;
38+
}
39+
40+
/**
41+
* The cases the issue names, plus the deliberate NON-rejections. `accepted`
42+
* is the verdict for BOTH moments — that equivalence is the point (see the
43+
* parity test at the bottom), so the table carries one column, not two.
44+
*/
45+
const CASES: Array<{ label: string; param: Record<string, unknown>; accepted: boolean }> = [
46+
// ── The hole, per type ────────────────────────────────────────────────────
47+
{
48+
label: "datetime + a wall-clock literal (the issue's example)",
49+
param: { name: 'start', type: 'datetime', defaultValue: '2026-08-10T15:00' },
50+
accepted: false,
51+
},
52+
{ label: "number + 'abc'", param: { name: 'qty', type: 'number', defaultValue: 'abc' }, accepted: false },
53+
{
54+
label: 'select + a value not in its own options',
55+
param: {
56+
name: 'tier',
57+
type: 'select',
58+
options: [{ label: 'Gold', value: 'gold' }, { label: 'Silver', value: 'silver' }],
59+
defaultValue: 'platinum',
60+
},
61+
accepted: false,
62+
},
63+
{
64+
label: 'multiple: true + a scalar default',
65+
param: { name: 'owners', type: 'user', multiple: true, defaultValue: 'usr_1' },
66+
accepted: false,
67+
},
68+
{
69+
label: 'date + a full instant (the mirror of the datetime case)',
70+
param: { name: 'due', type: 'date', defaultValue: '2026-08-10T15:00:00.000Z' },
71+
accepted: false,
72+
},
73+
{ label: 'boolean + a string', param: { name: 'notify', type: 'boolean', defaultValue: 'yes' }, accepted: false },
74+
{
75+
label: 'lookup + an embedded record object instead of an id',
76+
param: { name: 'owner', type: 'lookup', reference: 'sys_user', defaultValue: { id: 'usr_1', name: 'Ada' } },
77+
accepted: false,
78+
},
79+
80+
// ── Valid defaults of each type: untouched ────────────────────────────────
81+
{
82+
label: 'VALID datetime (ISO instant with zone)',
83+
param: { name: 'start', type: 'datetime', defaultValue: '2026-08-10T15:00:00.000Z' },
84+
accepted: true,
85+
},
86+
{ label: 'VALID number', param: { name: 'qty', type: 'number', defaultValue: 7 }, accepted: true },
87+
{
88+
label: 'VALID select member',
89+
param: { name: 'tier', type: 'select', options: [{ label: 'Gold', value: 'gold' }], defaultValue: 'gold' },
90+
accepted: true,
91+
},
92+
{
93+
label: 'VALID multiple array',
94+
param: { name: 'owners', type: 'user', multiple: true, defaultValue: ['usr_1'] },
95+
accepted: true,
96+
},
97+
{ label: 'VALID date', param: { name: 'due', type: 'date', defaultValue: '2026-08-10' }, accepted: true },
98+
{ label: 'VALID boolean', param: { name: 'notify', type: 'boolean', defaultValue: true }, accepted: true },
99+
{
100+
label: 'json — an explicitly OPEN value contract, so any default rides',
101+
param: { name: 'blob', type: 'json', defaultValue: { anything: ['at', 'all'] } },
102+
accepted: true,
103+
},
104+
{
105+
label: 'no `type` — the value shape is unresolvable, so it stays open',
106+
param: { name: 'loose', defaultValue: 'whatever' },
107+
accepted: true,
108+
},
109+
110+
// ── Presence parity: the dispatcher treats these as ABSENT ────────────────
111+
{
112+
label: "empty-string default on a datetime — ABSENT at submit, so not judged here either",
113+
param: { name: 'start', type: 'datetime', defaultValue: '' },
114+
accepted: true,
115+
},
116+
{
117+
label: 'null default on a number — ABSENT at submit',
118+
param: { name: 'qty', type: 'number', defaultValue: null },
119+
accepted: true,
120+
},
121+
];
122+
123+
describe('#6970 ActionParamSchema.defaultValue — authored defaults meet the param value contract', () => {
124+
for (const { label, param, accepted } of CASES) {
125+
it(`${accepted ? 'accepts' : 'rejects'}: ${label}`, () => {
126+
const issue = defaultValueIssue(param);
127+
if (accepted) {
128+
expect(issue).toBeNull();
129+
return;
130+
}
131+
// Rejection pin: this is a pure Zod parse (no error envelope), so the
132+
// assertion set is issue PATH + message shape — never a bare
133+
// `success === false`, which cannot tell this rejection from the
134+
// schema refusing the param for some unrelated reason.
135+
expect(issue).not.toBeNull();
136+
expect(issue!.path).toEqual(['defaultValue']);
137+
expect(issue!.code).toBe('custom');
138+
// Names the param, its type, and the offending literal — the three
139+
// things the submit-time 400 could not say.
140+
expect(issue!.message).toContain(`"${param.name as string}"`);
141+
expect(issue!.message).toContain(`(${param.type as string})`);
142+
expect(issue!.message).toContain(JSON.stringify(param.defaultValue));
143+
});
144+
}
145+
146+
it("names the author's default as the cause, not just the param", () => {
147+
const issue = defaultValueIssue({ name: 'start', type: 'datetime', defaultValue: '2026-08-10T15:00' })!;
148+
// The underlying reason is carried verbatim from the shared value contract,
149+
// so authoring and submit read identically.
150+
expect(issue.message).toContain('expected an ISO-8601 instant with explicit zone');
151+
expect(issue.message).toContain('cannot satisfy this param');
152+
expect(issue.message).toContain('PREFILL');
153+
});
154+
155+
it('reports through the real authoring door with a full params path', () => {
156+
const r = getMetadataTypeSchema('action')!.safeParse({
157+
name: 'schedule_visit',
158+
label: 'Schedule Visit',
159+
type: 'script',
160+
params: [
161+
{ name: 'note', type: 'text' },
162+
{ name: 'start', type: 'datetime', defaultValue: '2026-08-10T15:00' },
163+
],
164+
});
165+
expect(r.success).toBe(false);
166+
const paths = r.error!.issues.map((i) => i.path.join('.'));
167+
expect(paths).toContain('params.1.defaultValue');
168+
});
169+
170+
// ── The design's two deliberate non-rejections ────────────────────────────
171+
it('does NOT judge arity it cannot know — a field-backed param inherits `multiple`', () => {
172+
// `{ field: 'owners' }` inherits `multiple: true` from the referenced
173+
// field, which is invisible at parse time. Rejecting this array would be
174+
// the authoring gate guessing, and guessing wrong rejects valid metadata.
175+
expect(defaultValueIssue({ field: 'owners', type: 'user', defaultValue: ['usr_1', 'usr_2'] })).toBeNull();
176+
// The scalar spelling of the same inherited-arity param is equally legal.
177+
expect(defaultValueIssue({ field: 'owners', type: 'user', defaultValue: 'usr_1' })).toBeNull();
178+
});
179+
180+
it('still judges arity when the param STATES it, field-backed or not', () => {
181+
// `multiple` declared → the declaration answers the question, so it binds.
182+
expect(defaultValueIssue({ field: 'owners', type: 'user', multiple: true, defaultValue: 'usr_1' }))
183+
.not.toBeNull();
184+
// Inline (no `field`) → nothing to inherit from, so silence means scalar.
185+
expect(defaultValueIssue({ name: 'owners', type: 'user', defaultValue: ['usr_1'] })).not.toBeNull();
186+
});
187+
188+
it('does NOT judge option membership it cannot know — inherited option sets stay open', () => {
189+
// No inline `options`: the set comes from the referenced field, so
190+
// `valueSchemaFor` degrades to free-form and any string default rides.
191+
expect(defaultValueIssue({ field: 'tier', type: 'select', defaultValue: 'gold' })).toBeNull();
192+
});
193+
194+
it('leaves params WITHOUT a defaultValue completely untouched', () => {
195+
for (const type of ['datetime', 'number', 'select', 'user', 'date', 'boolean']) {
196+
expect(ActionParamSchema.safeParse({ name: 'x', type }).success).toBe(true);
197+
}
198+
});
199+
200+
/**
201+
* The ruling's actual claim: `defaultValue` goes through the SAME
202+
* `valueSchemaFor` machinery the dispatcher uses — no second rule set. This
203+
* is the pin that would catch a future edit re-implementing the check by
204+
* hand, which is how the two ends drift into two dialects.
205+
*/
206+
it('agrees with the dispatcher on every case — one rule set, two moments', () => {
207+
for (const { label, param } of CASES) {
208+
// Arity/membership the AUTHORING side deliberately cannot resolve are
209+
// excluded: the dispatcher is fed the RESOLVED param, so for those rows
210+
// the two sides are answering different questions by design.
211+
if (param.field !== undefined) continue;
212+
const authoringRejects = defaultValueIssue(param) !== null;
213+
const submitRejects = submitIssue(param) !== null;
214+
expect(
215+
{ case: label, authoringRejects },
216+
`authoring and submit must agree for: ${label}`,
217+
).toEqual({ case: label, authoringRejects: submitRejects });
218+
}
219+
});
220+
});

packages/spec/src/ui/action-params.zod.ts

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -114,10 +114,24 @@ const BUILTIN_PARAM_ORIGINS: ReadonlyMap<string, string> = new Map([
114114
/** Fallback origin sentence for a built-in supplied via `opts.builtinKeys`. */
115115
const GENERIC_BUILTIN_ORIGIN = 'the dispatcher supplies it.';
116116

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

133+
const isPresent = isActionParamValuePresent;
134+
121135
/**
122136
* The tail appended to an `unknown_field` message when the rejected key is one
123137
* leading underscore away from a built-in — `''` when it is not (an ordinary

0 commit comments

Comments
 (0)