From 6c07299f6d0896db74edcdc39b75830c61d74887 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 28 Jul 2026 02:33:52 +0000 Subject: [PATCH 1/2] fix(field): fold deprecated `conditionalRequired` into `requiredWhen` and drop the alias (#3754) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. That is exactly the condition that produced #3713, where the spec kept `target` while objectui's ActionRunner preferred the alias and one button ran two different scripts, client vs server. 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 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, not just a bare `FieldSchema.parse()`. This fixes no live bug: 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, removing the chance rather than a defect. objectql's fallback is deliberately KEPT: `evaluateValidationRules` is also handed raw, unparsed field definitions (see its own `conditionalRequired` test), which still carry the alias. Commented in place so it doesn't read as dead code. Authoring is unchanged — the alias is still accepted on input, still lowered, still documented. Consumers of the PARSED metadata must read `requiredWhen`; `z.infer` 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 — distinct from the pre-existing `FieldInput` factory type (`Partial`), which is unrelated. Verified: spec 6719, objectql 1119, runtime 655, metadata 276, lint 471, `pnpm build` 71/71, plus the liveness and generated-docs gates. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01JDwRktLBFwgtV5Svxm4LJm --- .changeset/field-conditional-required-fold.md | 47 +++++++++++++++ content/docs/data-modeling/field-types.mdx | 2 +- content/docs/data-modeling/fields.mdx | 7 ++- .../docs/data-modeling/validation-rules.mdx | 2 +- content/docs/references/data/field.mdx | 4 +- .../objectql/src/validation/rule-validator.ts | 8 ++- packages/spec/liveness/field.json | 3 +- packages/spec/src/data/field.test.ts | 59 +++++++++++++++---- packages/spec/src/data/field.zod.ts | 37 +++++++++++- 9 files changed, 148 insertions(+), 21 deletions(-) create mode 100644 .changeset/field-conditional-required-fold.md diff --git a/.changeset/field-conditional-required-fold.md b/.changeset/field-conditional-required-fold.md new file mode 100644 index 0000000000..1e802db59d --- /dev/null +++ b/.changeset/field-conditional-required-fold.md @@ -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` no longer carries `conditionalRequired`, so a stale +reader fails to compile rather than silently reading `undefined`. A new +`FieldParseInput` (`z.input`) names the author-facing shape that +still accepts the alias — distinct from the pre-existing `FieldInput` factory-helper +type, which is `Partial` and unrelated. diff --git a/content/docs/data-modeling/field-types.mdx b/content/docs/data-modeling/field-types.mdx index 987c5dc358..9bf4a62dce 100644 --- a/content/docs/data-modeling/field-types.mdx +++ b/content/docs/data-modeling/field-types.mdx @@ -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 | --- diff --git a/content/docs/data-modeling/fields.mdx b/content/docs/data-modeling/fields.mdx index 546a76b0d0..14f82d20e4 100644 --- a/content/docs/data-modeling/fields.mdx +++ b/content/docs/data-modeling/fields.mdx @@ -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 @@ -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 diff --git a/content/docs/data-modeling/validation-rules.mdx b/content/docs/data-modeling/validation-rules.mdx index acbae82d87..5576569af5 100644 --- a/content/docs/data-modeling/validation-rules.mdx +++ b/content/docs/data-modeling/validation-rules.mdx @@ -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 | --- diff --git a/content/docs/references/data/field.mdx b/content/docs/references/data/field.mdx index 985cc4ed78..a56298ca12 100644 --- a/content/docs/references/data/field.mdx +++ b/content/docs/references/data/field.mdx @@ -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:`) 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. | diff --git a/packages/objectql/src/validation/rule-validator.ts b/packages/objectql/src/validation/rule-validator.ts index 46974f90a8..53c64dda1e 100644 --- a/packages/objectql/src/validation/rule-validator.ts +++ b/packages/objectql/src/validation/rule-validator.ts @@ -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; diff --git a/packages/spec/liveness/field.json b/packages/spec/liveness/field.json index 8098ae0fa2..782941738e 100644 --- a/packages/spec/liveness/field.json +++ b/packages/spec/liveness/field.json @@ -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", diff --git a/packages/spec/src/data/field.test.ts b/packages/spec/src/data/field.test.ts index cb2ea8afef..7da9f8b1d2 100644 --- a/packages/spec/src/data/field.test.ts +++ b/packages/spec/src/data/field.test.ts @@ -1,7 +1,7 @@ import { describe, it, expect } from 'vitest'; -import { - FieldSchema, - FieldType, +import { + FieldSchema, + FieldType, SelectOptionSchema, CurrencyConfigSchema, CurrencyValueSchema, @@ -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', () => { @@ -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', () => { @@ -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; + expect(amount.requiredWhen).toEqual({ dialect: 'cel', source: "record.stage == 'closing'" }); + expect('conditionalRequired' in amount).toBe(false); }); }); @@ -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); }); }); diff --git a/packages/spec/src/data/field.zod.ts b/packages/spec/src/data/field.zod.ts index 18b698f727..353a38d1fd 100644 --- a/packages/spec/src/data/field.zod.ts +++ b/packages/spec/src/data/field.zod.ts @@ -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 @@ -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; +/** + * 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`. + */ +export type FieldParseInput = z.input; export type SelectOption = z.infer; export type LocationCoordinates = z.infer; export type Address = z.infer; From 1410e6392ee7a0fc0e6f77d3bea24a661150d04e Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 28 Jul 2026 02:42:43 +0000 Subject: [PATCH 2/2] chore(spec): record `FieldParseInput` in the public API-surface snapshot MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CI's api-surface gate (run inside the "TypeScript Type Check" job) flagged the new export: "0 breaking (removed/narrowed), 1 added". The addition is intentional — `FieldParseInput` = `z.input` names the author-facing shape that still accepts the deprecated `conditionalRequired` alias, mirroring the existing `CurrencyConfigInput` / `DataQualityRulesInput` in the same file and `ActionInput` on the action schema. Regenerated the snapshot per the gate's own instruction. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01JDwRktLBFwgtV5Svxm4LJm --- packages/spec/api-surface.json | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/spec/api-surface.json b/packages/spec/api-surface.json index fb4a96d31f..a6c249e3f4 100644 --- a/packages/spec/api-surface.json +++ b/packages/spec/api-surface.json @@ -339,6 +339,7 @@ "FieldNodeSchema (const)", "FieldOperators (type)", "FieldOperatorsSchema (const)", + "FieldParseInput (type)", "FieldReference (type)", "FieldReferenceSchema (const)", "FieldSchema (const)",