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
47 changes: 47 additions & 0 deletions .changeset/field-conditional-required-fold.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
---
"@objectstack/spec": minor
---

fix(field): fold the deprecated `conditionalRequired` alias into `requiredWhen` and drop it from the parsed output (#3754)

Second instance of the alias-drift shape #3713/#3742 fixed for `action.execute`.
`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, so every consumer had to re-implement the precedence. That is
exactly the condition that produced #3713, where the server kept `target` while
objectui's renderer preferred the alias and one button ran two different scripts.

Worse, the alias surviving parse was **test-pinned**, including a case literally
named *"requiredWhen and its alias conditionalRequired can coexist"* — the inverse
of the contract #3742 had just established one field over.

`FieldSchema` now lowers `conditionalRequired` into `requiredWhen` at parse time and
removes the alias from its output; `requiredWhen` wins when both are declared. The
pinning tests are inverted accordingly, and a new case asserts the alias is gone
from a field parsed through `ObjectSchema` — the path a renderer actually receives,
not just a bare `FieldSchema.parse()`.

No live bug is being fixed here: every reader we can see already prefers the
canonical key (`rule-validator.ts` reads `requiredWhen ?? conditionalRequired`). The
point is that nothing in the contract *made* that right. This is hardening — it
removes the chance rather than a defect.

`objectql`'s `requiredWhen ?? conditionalRequired` fallback is kept on purpose:
`evaluateValidationRules` is also handed raw, unparsed field definitions, which still
carry the alias.

**Authoring is unchanged.** `conditionalRequired` is still accepted on input, still
lowered, still listed in the reference docs and JSON Schema. Nothing to migrate in
app metadata.

**Consumers of the parsed metadata** must read the canonical slot:

- FROM `parsedField.conditionalRequired` → TO `parsedField.requiredWhen`
- One-line fix: `field.conditionalRequired || field.requiredWhen` becomes
`field.requiredWhen`

`z.infer<typeof FieldSchema>` no longer carries `conditionalRequired`, so a stale
reader fails to compile rather than silently reading `undefined`. A new
`FieldParseInput` (`z.input<typeof FieldSchema>`) names the author-facing shape that
still accepts the alias — distinct from the pre-existing `FieldInput` factory-helper
type, which is `Partial<Field>` and unrelated.
2 changes: 1 addition & 1 deletion content/docs/data-modeling/field-types.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -642,7 +642,7 @@ These properties are available on **all** field types:
| `visibleWhen` | `Expression` | — | Show field only when predicate is true |
| `readonlyWhen` | `Expression` | — | Make field read-only when predicate is true |
| `requiredWhen` | `Expression` | — | Require field when predicate is true |
| `conditionalRequired` | `Expression` | — | Deprecated alias of `requiredWhen` |
| `conditionalRequired` | `Expression` | — | Deprecated alias of `requiredWhen` — lowered into it at parse time, then dropped from the parsed metadata |

---

Expand Down
7 changes: 5 additions & 2 deletions content/docs/data-modeling/fields.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -314,7 +314,7 @@ For values that must be encrypted at rest (API keys, tokens, DB passwords), use
| `visibleWhen` | `string \| Expression` | CEL predicate; field is shown only when `TRUE` |
| `readonlyWhen` | `string \| Expression` | CEL predicate; field is read-only when `TRUE` |
| `requiredWhen` | `string \| Expression` | CEL predicate; field is required when `TRUE` |
| `conditionalRequired` | `string \| Expression` | Deprecated alias of `requiredWhen` |
| `conditionalRequired` | `string \| Expression` | Deprecated alias of `requiredWhen` — lowered into it at parse time, then dropped |
| `multiple` | `boolean` | Allow multiple values (for select, lookup, file) |

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

## Naming Conventions

Expand Down
2 changes: 1 addition & 1 deletion content/docs/data-modeling/validation-rules.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ These properties apply to **all** field types and are validated by the base `Fie
| `visibleWhen` | `string \| Expression` | — | CEL predicate; field is shown only when `TRUE` |
| `readonlyWhen` | `string \| Expression` | — | CEL predicate; field is read-only when `TRUE` |
| `requiredWhen` | `string \| Expression` | — | CEL predicate; field is required when `TRUE` |
| `conditionalRequired` | `string \| Expression` | — | Deprecated alias of `requiredWhen` |
| `conditionalRequired` | `string \| Expression` | — | Deprecated alias of `requiredWhen` — lowered into it at parse time, then dropped from the parsed metadata |

---

Expand Down
4 changes: 2 additions & 2 deletions content/docs/references/data/field.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -143,8 +143,8 @@ const result = Address.parse(data);
| **group** | `string` | optional | Field group name for organizing fields in forms and layouts (e.g., "contact_info", "billing", "system") |
| **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'` |
| **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'` |
| **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`. |
| **conditionalRequired** | `string \| { dialect: Enum<'cel' \| 'cron' \| 'template'>; source?: string; ast?: any; meta?: object }` | optional | Predicate (CEL) — field is required when TRUE. Alias of `requiredWhen`. |
| **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. |
| **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. |
| **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". |
| **hidden** | `boolean` | optional | Hidden from default UI |
| **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. |
Expand Down
8 changes: 7 additions & 1 deletion packages/objectql/src/validation/rule-validator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -421,7 +421,13 @@ interface ConditionalFieldOption {

interface ConditionalFieldDef {
requiredWhen?: string | Expression;
conditionalRequired?: string | Expression; // back-compat alias of requiredWhen
// Back-compat alias of `requiredWhen`. Since #3754 `FieldSchema`'s transform
// lowers this into `requiredWhen` and drops it, so PARSED metadata never carries
// it — but the alias-reading fallback below stays deliberately: this validator is
// also handed RAW, unparsed field definitions (see the `conditionalRequired`
// case in rule-validator.test.ts), and those still need honouring. Canonical
// first, always — never `conditionalRequired ?? requiredWhen`.
conditionalRequired?: string | Expression;
readonlyWhen?: string | Expression;
/** Static, unconditional read-only flag (`field.readonly`). #2948. */
readonly?: boolean;
Expand Down
1 change: 1 addition & 0 deletions packages/spec/api-surface.json
Original file line number Diff line number Diff line change
Expand Up @@ -339,6 +339,7 @@
"FieldNodeSchema (const)",
"FieldOperators (type)",
"FieldOperatorsSchema (const)",
"FieldParseInput (type)",
"FieldReference (type)",
"FieldReferenceSchema (const)",
"FieldSchema (const)",
Expand Down
3 changes: 2 additions & 1 deletion packages/spec/liveness/field.json
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,8 @@
},
"conditionalRequired": {
"status": "live",
"note": "deprecated alias of requiredWhen — plan removal."
"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",
"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."
},
"readonly": {
"status": "live",
Expand Down
59 changes: 49 additions & 10 deletions packages/spec/src/data/field.test.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { describe, it, expect } from 'vitest';
import {
FieldSchema,
FieldType,
import {
FieldSchema,
FieldType,
SelectOptionSchema,
CurrencyConfigSchema,
CurrencyValueSchema,
Expand All @@ -10,6 +10,7 @@ import {
type CurrencyConfig,
type CurrencyValue,
} from './field.zod';
import { ObjectSchema } from './object.zod';

describe('FieldType', () => {
it('should accept valid field types', () => {
Expand Down Expand Up @@ -940,20 +941,26 @@ describe('FieldSchema - group property', () => {
});
});

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

it('should accept a field without conditionalRequired (optional)', () => {
const result = FieldSchema.parse({
type: 'text',
});
expect(result.conditionalRequired).toBeUndefined();
expect(result.requiredWhen).toBeUndefined();
expect('conditionalRequired' in result).toBe(false);
});

it('should allow combining required and conditionalRequired', () => {
Expand All @@ -963,7 +970,35 @@ describe('FieldSchema - conditionalRequired property', () => {
conditionalRequired: 'amount > 1000',
});
expect(result.required).toBe(false);
expect(result.conditionalRequired).toEqual({ dialect: 'cel', source: 'amount > 1000' });
expect(result.requiredWhen).toEqual({ dialect: 'cel', source: 'amount > 1000' });
expect('conditionalRequired' in result).toBe(false);
});

it('keeps requiredWhen when BOTH are declared, and drops the alias', () => {
const result = FieldSchema.parse({
type: 'text',
requiredWhen: "record.status == 'sent'",
conditionalRequired: 'record.amount > 1000',
});
expect(result.requiredWhen).toEqual({ dialect: 'cel', source: "record.status == 'sent'" });
expect('conditionalRequired' in result).toBe(false);
expect(Object.keys(result)).not.toContain('conditionalRequired');
expect(JSON.parse(JSON.stringify(result)).conditionalRequired).toBeUndefined();
});

it('survives a field nested in an object (the real authoring path)', () => {
// The alias has to be gone from the metadata a renderer actually receives,
// not just from a bare FieldSchema.parse() in a unit test.
const parsed = ObjectSchema.parse({
name: 'crm_deal',
label: 'Deal',
fields: {
amount: { type: 'currency', conditionalRequired: "record.stage == 'closing'" },
},
});
const amount = parsed.fields.amount as Record<string, unknown>;
expect(amount.requiredWhen).toEqual({ dialect: 'cel', source: "record.stage == 'closing'" });
expect('conditionalRequired' in amount).toBe(false);
});
});

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

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

Expand Down
37 changes: 34 additions & 3 deletions packages/spec/src/data/field.zod.ts
Original file line number Diff line number Diff line change
Expand Up @@ -557,11 +557,14 @@ export const FieldSchema = lazySchema(() => z.object({
*/
visibleWhen: ExpressionInputSchema.optional().describe("Predicate (CEL) — field is shown only when TRUE (else hidden). e.g. P`record.type == 'invoice'`"),
readonlyWhen: ExpressionInputSchema.optional().describe("Predicate (CEL) — field is read-only when TRUE. e.g. P`record.status == 'paid'`"),
requiredWhen: ExpressionInputSchema.optional().describe("Predicate (CEL) — field is required when TRUE. Canonical name for `conditionalRequired`."),
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."),

/** Conditional Requirements
* @deprecated Alias of `requiredWhen` — kept for back-compat. */
conditionalRequired: ExpressionInputSchema.optional().describe('Predicate (CEL) — field is required when TRUE. Alias of `requiredWhen`.'),
* @deprecated Alias of `requiredWhen`. Accepted on input, lowered into
* `requiredWhen` at parse time, then **removed from the parsed output**
* (#3754) — so every consumer reads one canonical slot. When both are set,
* `requiredWhen` wins and this value is discarded. */
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.'),

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

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