Skip to content

Commit f5a2320

Browse files
authored
fix(field): fold deprecated conditionalRequired into requiredWhen and drop the alias (#3754) (#3764)
Second instance of the alias-drift shape #3742 fixed for `action.execute`, found while verifying that fix. `requiredWhen` is canonical and `conditionalRequired` is its documented deprecated alias, but `FieldSchema` had NO canonicalization at all: both keys stayed live in the parsed output, leaving every consumer to re-implement the precedence — the condition that produced #3713. The alias surviving parse was even test-pinned, including a case named "requiredWhen and its alias conditionalRequired can coexist", the inverse of the contract #3742 had just established one field over. `FieldSchema` now lowers the alias into `requiredWhen` at parse time and removes it from the output; canonical wins when both are declared. The pinning tests are inverted, and a new case asserts the alias is gone from a field parsed through `ObjectSchema` — the path a renderer actually receives. This fixes no live bug: every reader we can see already prefers the canonical key (`rule-validator.ts` reads `requiredWhen ?? conditionalRequired`). Nothing in the contract MADE that right — this is hardening, removing the chance rather than a defect. objectql's fallback is deliberately kept: `evaluateValidationRules` is also handed raw, unparsed field definitions, which still carry the alias. Authoring is unchanged. Consumers of the parsed metadata must read `requiredWhen`; `z.infer<typeof FieldSchema>` no longer carries the alias, so a stale reader fails to compile instead of silently reading `undefined`. Adds `FieldParseInput` (`z.input`) for the author-facing shape — `FieldInput` was already taken by an unrelated `Partial<Field>` factory type in the same file. Closes #3754.
1 parent 677b591 commit f5a2320

10 files changed

Lines changed: 149 additions & 21 deletions

File tree

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
---
2+
"@objectstack/spec": minor
3+
---
4+
5+
fix(field): fold the deprecated `conditionalRequired` alias into `requiredWhen` and drop it from the parsed output (#3754)
6+
7+
Second instance of the alias-drift shape #3713/#3742 fixed for `action.execute`.
8+
`requiredWhen` is canonical and `conditionalRequired` is its documented deprecated
9+
alias, but `FieldSchema` had **no canonicalization at all** — both keys stayed live
10+
in the parsed output, so every consumer had to re-implement the precedence. That is
11+
exactly the condition that produced #3713, where the server kept `target` while
12+
objectui's renderer preferred the alias and one button ran two different scripts.
13+
14+
Worse, the alias surviving parse was **test-pinned**, including a case literally
15+
named *"requiredWhen and its alias conditionalRequired can coexist"* — the inverse
16+
of the contract #3742 had just established one field over.
17+
18+
`FieldSchema` now lowers `conditionalRequired` into `requiredWhen` at parse time and
19+
removes the alias from its output; `requiredWhen` wins when both are declared. The
20+
pinning tests are inverted accordingly, and a new case asserts the alias is gone
21+
from a field parsed through `ObjectSchema` — the path a renderer actually receives,
22+
not just a bare `FieldSchema.parse()`.
23+
24+
No live bug is being fixed here: every reader we can see already prefers the
25+
canonical key (`rule-validator.ts` reads `requiredWhen ?? conditionalRequired`). The
26+
point is that nothing in the contract *made* that right. This is hardening — it
27+
removes the chance rather than a defect.
28+
29+
`objectql`'s `requiredWhen ?? conditionalRequired` fallback is kept on purpose:
30+
`evaluateValidationRules` is also handed raw, unparsed field definitions, which still
31+
carry the alias.
32+
33+
**Authoring is unchanged.** `conditionalRequired` is still accepted on input, still
34+
lowered, still listed in the reference docs and JSON Schema. Nothing to migrate in
35+
app metadata.
36+
37+
**Consumers of the parsed metadata** must read the canonical slot:
38+
39+
- FROM `parsedField.conditionalRequired` → TO `parsedField.requiredWhen`
40+
- One-line fix: `field.conditionalRequired || field.requiredWhen` becomes
41+
`field.requiredWhen`
42+
43+
`z.infer<typeof FieldSchema>` no longer carries `conditionalRequired`, so a stale
44+
reader fails to compile rather than silently reading `undefined`. A new
45+
`FieldParseInput` (`z.input<typeof FieldSchema>`) names the author-facing shape that
46+
still accepts the alias — distinct from the pre-existing `FieldInput` factory-helper
47+
type, which is `Partial<Field>` and unrelated.

‎content/docs/data-modeling/field-types.mdx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -642,7 +642,7 @@ These properties are available on **all** field types:
642642
| `visibleWhen` | `Expression` | — | Show field only when predicate is true |
643643
| `readonlyWhen` | `Expression` | — | Make field read-only when predicate is true |
644644
| `requiredWhen` | `Expression` | — | Require field when predicate is true |
645-
| `conditionalRequired` | `Expression` | — | Deprecated alias of `requiredWhen` |
645+
| `conditionalRequired` | `Expression` | — | Deprecated alias of `requiredWhen` — lowered into it at parse time, then dropped from the parsed metadata |
646646
647647
---
648648

‎content/docs/data-modeling/fields.mdx‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -314,7 +314,7 @@ For values that must be encrypted at rest (API keys, tokens, DB passwords), use
314314
| `visibleWhen` | `string \| Expression` | CEL predicate; field is shown only when `TRUE` |
315315
| `readonlyWhen` | `string \| Expression` | CEL predicate; field is read-only when `TRUE` |
316316
| `requiredWhen` | `string \| Expression` | CEL predicate; field is required when `TRUE` |
317-
| `conditionalRequired` | `string \| Expression` | Deprecated alias of `requiredWhen` |
317+
| `conditionalRequired` | `string \| Expression` | Deprecated alias of `requiredWhen` — lowered into it at parse time, then dropped |
318318
| `multiple` | `boolean` | Allow multiple values (for select, lookup, file) |
319319

320320
```typescript
@@ -344,7 +344,10 @@ changes, including row-scoped rules inside inline master-detail grids. The
344344
server enforces `requiredWhen` on submit and ignores writes to fields whose
345345
`readonlyWhen` predicate is `TRUE`, so client-side affordances are not the
346346
only guard. Use `requiredWhen` for new metadata; keep `conditionalRequired`
347-
only for older packages that already emitted it.
347+
only for older packages that already emitted it. The alias is still accepted on
348+
input, but it is lowered into `requiredWhen` at parse time and then removed from
349+
the parsed metadata, so every consumer reads one canonical slot — if a field
350+
declares both, `requiredWhen` wins and the alias is discarded.
348351

349352
## Naming Conventions
350353

‎content/docs/data-modeling/validation-rules.mdx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ These properties apply to **all** field types and are validated by the base `Fie
3434
| `visibleWhen` | `string \| Expression` | — | CEL predicate; field is shown only when `TRUE` |
3535
| `readonlyWhen` | `string \| Expression` | — | CEL predicate; field is read-only when `TRUE` |
3636
| `requiredWhen` | `string \| Expression` | — | CEL predicate; field is required when `TRUE` |
37-
| `conditionalRequired` | `string \| Expression` | — | Deprecated alias of `requiredWhen` |
37+
| `conditionalRequired` | `string \| Expression` | — | Deprecated alias of `requiredWhen` — lowered into it at parse time, then dropped from the parsed metadata |
3838

3939
---
4040

‎content/docs/references/data/field.mdx‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -117,8 +117,8 @@ const result = Address.parse(data);
117117
| **group** | `string` | optional | Field group name for organizing fields in forms and layouts (e.g., "contact_info", "billing", "system") |
118118
| **visibleWhen** | `string \| { dialect: Enum<'cel' \| 'cron' \| 'template'>; source?: string; ast?: any; meta?: object }` | optional | Predicate (CEL) — field is shown only when TRUE (else hidden). e.g. P`record.type == 'invoice'` |
119119
| **readonlyWhen** | `string \| { dialect: Enum<'cel' \| 'cron' \| 'template'>; source?: string; ast?: any; meta?: object }` | optional | Predicate (CEL) — field is read-only when TRUE. e.g. P`record.status == 'paid'` |
120-
| **requiredWhen** | `string \| { dialect: Enum<'cel' \| 'cron' \| 'template'>; source?: string; ast?: any; meta?: object }` | optional | Predicate (CEL) — field is required when TRUE. Canonical name for `conditionalRequired`. |
121-
| **conditionalRequired** | `string \| { dialect: Enum<'cel' \| 'cron' \| 'template'>; source?: string; ast?: any; meta?: object }` | optional | Predicate (CEL) — field is required when TRUE. Alias of `requiredWhen`. |
120+
| **requiredWhen** | `string \| { dialect: Enum<'cel' \| 'cron' \| 'template'>; source?: string; ast?: any; meta?: object }` | optional | Predicate (CEL) — field is required when TRUE. Canonical slot; the deprecated `conditionalRequired` alias is lowered into this one at parse time. |
121+
| **conditionalRequired** | `string \| { dialect: Enum<'cel' \| 'cron' \| 'template'>; source?: string; ast?: any; meta?: object }` | optional | @deprecated — Use requiredWhen instead. Lowered into requiredWhen during parsing and dropped from the parsed output; when both are set, requiredWhen wins. |
122122
| **widget** | `string` | optional | Form widget override — names a registered field component (resolved as `field:<widget>`) to render this field instead of the `type` default. Degrades to the `type` renderer when unregistered. e.g. "object-ref", "filter-condition", "recipient-picker". |
123123
| **hidden** | `boolean` | optional | Hidden from default UI |
124124
| **readonly** | `boolean` | optional | Read-only — never editable in forms, AND server-enforced on BOTH write paths: a non-system write to this field is silently dropped from the payload on UPDATE (#2948/#3003) and on INSERT (#3043; a create can no longer directly seed e.g. `approval_status: "approved"`), symmetric with `readonlyWhen`. A stripped INSERT field still falls back to its `defaultValue`. Exempt from the strip: `isSystem` writes (seed replay, migration), and an opt-in "historical" import (`preserveAudit`, #3493) — which admits a whitelist (the audit/timestamp family plus author-declared business `readonly` fields). A normal (non-system) import is NOT system-context and still strips. |

‎packages/objectql/src/validation/rule-validator.ts‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -421,7 +421,13 @@ interface ConditionalFieldOption {
421421

422422
interface ConditionalFieldDef {
423423
requiredWhen?: string | Expression;
424-
conditionalRequired?: string | Expression; // back-compat alias of requiredWhen
424+
// Back-compat alias of `requiredWhen`. Since #3754 `FieldSchema`'s transform
425+
// lowers this into `requiredWhen` and drops it, so PARSED metadata never carries
426+
// it — but the alias-reading fallback below stays deliberately: this validator is
427+
// also handed RAW, unparsed field definitions (see the `conditionalRequired`
428+
// case in rule-validator.test.ts), and those still need honouring. Canonical
429+
// first, always — never `conditionalRequired ?? requiredWhen`.
430+
conditionalRequired?: string | Expression;
425431
readonlyWhen?: string | Expression;
426432
/** Static, unconditional read-only flag (`field.readonly`). #2948. */
427433
readonly?: boolean;

‎packages/spec/api-surface.json‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -342,6 +342,7 @@
342342
"FieldNodeSchema (const)",
343343
"FieldOperators (type)",
344344
"FieldOperatorsSchema (const)",
345+
"FieldParseInput (type)",
345346
"FieldReference (type)",
346347
"FieldReferenceSchema (const)",
347348
"FieldSchema (const)",

‎packages/spec/liveness/field.json‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,8 @@
8383
},
8484
"conditionalRequired": {
8585
"status": "live",
86-
"note": "deprecated alias of requiredWhen — plan removal."
86+
"evidence": "packages/spec/src/data/field.zod.ts:639 — the FieldSchema .transform lowers conditionalRequired -> requiredWhen and DROPS the alias, so authoring it changes what the validator enforces",
87+
"note": "#3754: folded at parse time and removed from the parsed output (same shape as action.execute -> target in #3742, agent.knowledge.topics -> sources in #1891). Previously BOTH keys stayed live in the output with no canonicalization, so each consumer re-derived the precedence — the condition that produced the #3713 divergence. objectql rule-validator.ts keeps its `requiredWhen ?? conditionalRequired` fallback on purpose: raw (unparsed) metadata still reaches evaluateValidationRules. Removal of the input key is still the endgame."
8788
},
8889
"readonly": {
8990
"status": "live",

‎packages/spec/src/data/field.test.ts‎

Lines changed: 49 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import { describe, it, expect } from 'vitest';
2-
import {
3-
FieldSchema,
4-
FieldType,
2+
import {
3+
FieldSchema,
4+
FieldType,
55
SelectOptionSchema,
66
CurrencyConfigSchema,
77
CurrencyValueSchema,
@@ -10,6 +10,7 @@ import {
1010
type CurrencyConfig,
1111
type CurrencyValue,
1212
} from './field.zod';
13+
import { ObjectSchema } from './object.zod';
1314

1415
describe('FieldType', () => {
1516
it('should accept valid field types', () => {
@@ -940,20 +941,26 @@ describe('FieldSchema - group property', () => {
940941
});
941942
});
942943

943-
describe('FieldSchema - conditionalRequired property', () => {
944-
it('should accept a field with conditionalRequired formula', () => {
944+
describe('FieldSchema - conditionalRequired → requiredWhen migration (#3754)', () => {
945+
// These three cases used to assert that `conditionalRequired` SURVIVED parse,
946+
// which pinned the alias into the output and left every consumer to invent its
947+
// own precedence — the condition that produced #3713 for `action.execute`. The
948+
// alias is now lowered into the canonical `requiredWhen` and dropped.
949+
it('lowers a conditionalRequired formula into requiredWhen', () => {
945950
const result = FieldSchema.parse({
946951
type: 'text',
947952
conditionalRequired: "status = 'closed_won'",
948953
});
949-
expect(result.conditionalRequired).toEqual({ dialect: 'cel', source: "status = 'closed_won'" });
954+
expect(result.requiredWhen).toEqual({ dialect: 'cel', source: "status = 'closed_won'" });
955+
expect('conditionalRequired' in result).toBe(false);
950956
});
951957

952958
it('should accept a field without conditionalRequired (optional)', () => {
953959
const result = FieldSchema.parse({
954960
type: 'text',
955961
});
956-
expect(result.conditionalRequired).toBeUndefined();
962+
expect(result.requiredWhen).toBeUndefined();
963+
expect('conditionalRequired' in result).toBe(false);
957964
});
958965

959966
it('should allow combining required and conditionalRequired', () => {
@@ -963,7 +970,35 @@ describe('FieldSchema - conditionalRequired property', () => {
963970
conditionalRequired: 'amount > 1000',
964971
});
965972
expect(result.required).toBe(false);
966-
expect(result.conditionalRequired).toEqual({ dialect: 'cel', source: 'amount > 1000' });
973+
expect(result.requiredWhen).toEqual({ dialect: 'cel', source: 'amount > 1000' });
974+
expect('conditionalRequired' in result).toBe(false);
975+
});
976+
977+
it('keeps requiredWhen when BOTH are declared, and drops the alias', () => {
978+
const result = FieldSchema.parse({
979+
type: 'text',
980+
requiredWhen: "record.status == 'sent'",
981+
conditionalRequired: 'record.amount > 1000',
982+
});
983+
expect(result.requiredWhen).toEqual({ dialect: 'cel', source: "record.status == 'sent'" });
984+
expect('conditionalRequired' in result).toBe(false);
985+
expect(Object.keys(result)).not.toContain('conditionalRequired');
986+
expect(JSON.parse(JSON.stringify(result)).conditionalRequired).toBeUndefined();
987+
});
988+
989+
it('survives a field nested in an object (the real authoring path)', () => {
990+
// The alias has to be gone from the metadata a renderer actually receives,
991+
// not just from a bare FieldSchema.parse() in a unit test.
992+
const parsed = ObjectSchema.parse({
993+
name: 'crm_deal',
994+
label: 'Deal',
995+
fields: {
996+
amount: { type: 'currency', conditionalRequired: "record.stage == 'closing'" },
997+
},
998+
});
999+
const amount = parsed.fields.amount as Record<string, unknown>;
1000+
expect(amount.requiredWhen).toEqual({ dialect: 'cel', source: "record.stage == 'closing'" });
1001+
expect('conditionalRequired' in amount).toBe(false);
9671002
});
9681003
});
9691004

@@ -987,14 +1022,18 @@ describe('FieldSchema - conditional field rules (visibleWhen / readonlyWhen / re
9871022
expect(result.requiredWhen).toBeUndefined();
9881023
});
9891024

990-
it('requiredWhen and its alias conditionalRequired can coexist', () => {
1025+
it('requiredWhen and its alias conditionalRequired can NOT coexist — canonical wins, alias dropped (#3754)', () => {
1026+
// Inverted from the original assertion, which pinned the two keys as
1027+
// coexisting in the parsed output. Coexistence is the whole problem: it makes
1028+
// every consumer re-derive the precedence, which is how #3713 produced a
1029+
// button that ran one script on the server and a different one in the browser.
9911030
const result = FieldSchema.parse({
9921031
type: 'text',
9931032
requiredWhen: "record.status == 'sent'",
9941033
conditionalRequired: "record.status == 'closed_won'",
9951034
});
9961035
expect(result.requiredWhen).toEqual({ dialect: 'cel', source: "record.status == 'sent'" });
997-
expect(result.conditionalRequired).toEqual({ dialect: 'cel', source: "record.status == 'closed_won'" });
1036+
expect('conditionalRequired' in result).toBe(false);
9981037
});
9991038
});
10001039

‎packages/spec/src/data/field.zod.ts‎

Lines changed: 34 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -524,11 +524,14 @@ export const FieldSchema = lazySchema(() => z.object({
524524
*/
525525
visibleWhen: ExpressionInputSchema.optional().describe("Predicate (CEL) — field is shown only when TRUE (else hidden). e.g. P`record.type == 'invoice'`"),
526526
readonlyWhen: ExpressionInputSchema.optional().describe("Predicate (CEL) — field is read-only when TRUE. e.g. P`record.status == 'paid'`"),
527-
requiredWhen: ExpressionInputSchema.optional().describe("Predicate (CEL) — field is required when TRUE. Canonical name for `conditionalRequired`."),
527+
requiredWhen: ExpressionInputSchema.optional().describe("Predicate (CEL) — field is required when TRUE. Canonical slot; the deprecated `conditionalRequired` alias is lowered into this one at parse time."),
528528

529529
/** Conditional Requirements
530-
* @deprecated Alias of `requiredWhen` — kept for back-compat. */
531-
conditionalRequired: ExpressionInputSchema.optional().describe('Predicate (CEL) — field is required when TRUE. Alias of `requiredWhen`.'),
530+
* @deprecated Alias of `requiredWhen`. Accepted on input, lowered into
531+
* `requiredWhen` at parse time, then **removed from the parsed output**
532+
* (#3754) — so every consumer reads one canonical slot. When both are set,
533+
* `requiredWhen` wins and this value is discarded. */
534+
conditionalRequired: ExpressionInputSchema.optional().describe('@deprecated — Use requiredWhen instead. Lowered into requiredWhen during parsing and dropped from the parsed output; when both are set, requiredWhen wins.'),
532535

533536
/**
534537
* Form widget override. Names a registered field/UI component to render this
@@ -600,9 +603,37 @@ export const FieldSchema = lazySchema(() => z.object({
600603
// driver builds indexes from the object's `indexes[]` array; a field-level
601604
// `index: true` created no index. Declare the index in object `indexes[]`.
602605
externalId: z.boolean().default(false).describe('Is external ID for upsert operations'),
606+
}).transform((data) => {
607+
// #3754: lower the deprecated `conditionalRequired` alias into the canonical
608+
// `requiredWhen` and DROP it from the output — the shape #3713/#3742 settled on
609+
// for `action.execute` → `target`, and #1891 for `agent.knowledge.topics` →
610+
// `sources`. Canonical wins when both are set.
611+
//
612+
// Why this is worth doing even though today's readers all agree: leaving both
613+
// keys live in the parsed output means every consumer has to re-implement the
614+
// precedence, and that is exactly the condition that produced #3713 — there the
615+
// server kept `target` while objectui's ActionRunner preferred the alias, so one
616+
// button ran two different scripts. The server side here happens to get it right
617+
// (`rule-validator.ts` reads `requiredWhen ?? conditionalRequired`), but nothing
618+
// in the contract *made* it right. Folding once, here, removes the chance.
619+
//
620+
// Authors may still write `conditionalRequired` — it stays on the parse-input
621+
// type (`FieldParseInput`) and in the reference docs; only the output is canonical.
622+
const { conditionalRequired, ...rest } = data;
623+
if (conditionalRequired && !rest.requiredWhen) {
624+
return { ...rest, requiredWhen: conditionalRequired };
625+
}
626+
return rest;
603627
}));
604628

605629
export type Field = z.infer<typeof FieldSchema>;
630+
/**
631+
* Author-facing parse INPUT for a field. Unlike {@link Field} (the parsed output)
632+
* this still carries the deprecated `conditionalRequired` alias, which the schema
633+
* lowers into `requiredWhen` at parse time (#3754). Distinct from the
634+
* `FieldInput` factory-helper type further down, which is `Partial<Field>`.
635+
*/
636+
export type FieldParseInput = z.input<typeof FieldSchema>;
606637
export type SelectOption = z.infer<typeof SelectOptionSchema>;
607638
export type LocationCoordinates = z.infer<typeof LocationCoordinatesSchema>;
608639
export type Address = z.infer<typeof AddressSchema>;

0 commit comments

Comments
 (0)