Skip to content

[P2] field.conditionalRequired is the same alias-drift shape #3713 just fixed — and the spec pins the alias to SURVIVE parse #3754

Description

@os-zhuang

Found while fixing #3713 (action.execute vs target). Same bug class, one field over — and this one is arguably worse, because the spec has no canonicalization at all.

The shape

requiredWhen is canonical; conditionalRequired is its documented deprecated alias (packages/spec/src/data/field.zod.ts):

requiredWhen: ExpressionInputSchema.optional().describe("… Canonical name for `conditionalRequired`."),
/** @deprecated Alias of `requiredWhen` — kept for back-compat. */
conditionalRequired: ExpressionInputSchema.optional().describe('… Alias of `requiredWhen`.'),

Unlike action.execute, there is no transform folding it — no lowering, no dropping. Both keys reach every consumer, so the precedence is re-implemented per reader instead of being decided once in the spec. That is precisely the condition that produced #3713.

Worse, the alias surviving parse is test-pinned, i.e. the inverse of what #3713 just established for action.execute (packages/spec/src/data/field.test.ts):

expect(result.conditionalRequired).toEqual({ dialect: 'cel', source: "status = 'closed_won'" });

Current readers

Reader Behavior
packages/objectql/src/validation/rule-validator.ts f.requiredWhen || f.conditionalRequired — canonical first ✅
packages/lint/src/validate-expressions.ts iterates both keys to validate expressions — correct, order-independent ✅
Studio object designer (v15, #2571) rewrites conditionalRequiredrequiredWhen on first edit — UI-only, so code-authored metadata never passes through it
objectui client-side required rendering unverified (separate repo) — if it reads conditionalRequired || requiredWhen, this is #3713 verbatim: server enforces one predicate, the form marks required by another

So today the server direction is right, but nothing in the contract makes it right, and the one surface most likely to have picked the alias-first order is the same one that did in #3713.

Suggested fix

Apply the shape #3713 landed on, which is now the established pattern for this repo (and matches agent.knowledge.topicssources, #1891):

  1. Add a transform to FieldSchema that folds conditionalRequired into requiredWhen when the canonical key is absent, and drops the alias from the parsed output. Canonical wins when both are set.
  2. Invert the pinning tests: assert 'conditionalRequired' in parsed === false and that an alias-only field lands on requiredWhen.
  3. Simplify rule-validator.ts to read requiredWhen only — with the alias gone from parsed metadata, the || fallback is dead code (keep the raw-metadata path in mind if any caller bypasses parse).
  4. Authoring stays unchanged: conditionalRequired remains accepted on input (z.input), still documented, nothing to migrate in app metadata.

Note this is a minor for @objectstack/spec on the same reasoning as #3742 — the parsed output loses a deprecated key, so any TS consumer reading parsed.conditionalRequired fails to compile instead of silently reading undefined.

Worth a sweep, not just a fix

action.execute and field.conditionalRequired are two instances of one pattern: a deprecated alias declared in the spec but left live in the parsed output, leaving N consumers to each invent a precedence. The remaining @deprecated/alias declarations in packages/spec/src deserve the same audit — the ones that fold-and-drop are safe, the ones that merely coexist are latent #3713s. Related debt named in AGENTS.md Prime Directive #12: the cfg.filter ?? cfg.filters / cfg.objectName ?? cfg.object fallbacks in the flow executors.

Refs #3713, #3742, #1891, #2571.

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions