fix(field): fold deprecated conditionalRequired into requiredWhen and drop the alias (#3754) - #3764
Merged
Merged
Conversation
… and drop the alias (#3754) 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<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 — distinct from the pre-existing `FieldInput` factory type (`Partial<Field>`), 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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JDwRktLBFwgtV5Svxm4LJm
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
github-actions
Bot
added
documentation
Contributor
📓 Docs Drift CheckThis PR changes 2 package(s): 108 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:
|
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<typeof FieldSchema>` 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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JDwRktLBFwgtV5Svxm4LJm
os-zhuang
marked this pull request as ready for review
July 28, 2026 02:58
This was referenced Jul 28, 2026
os-zhuang
added a commit
that referenced
this pull request
Jul 28, 2026
…#3743) (#3854) #3838 introduced `lintDeprecatedAliases` — the pre-parse pass that reports an alias the parse itself is about to consume — with one rule, for `action.execute`. #3743 predicted the pass would earn its keep beyond that rule. It does: `execute` was never special. The spec has exactly THREE transforms that fold an alias into its canonical key and then drop it from the parsed output, and all three share one failure mode — declare both slots with different values and one is discarded with no signal, invisible to every downstream check because the parse already erased the evidence. Two more rules, same shape, same advisory severity, same two surfaces: field-requiredwhen-conditionalrequired-conflict — `FieldSchema` folds `conditionalRequired` into `requiredWhen` (#3754/#3764); the discarded predicate never gates the field. Covers fields on objects AND on object extensions, and compares the predicate TEXT so a bare string and the `{ dialect, source }` envelope it lowers into read as the same predicate. agent-knowledge-sources-topics-conflict — `AIKnowledgeSchema` folds `knowledge.topics` into `knowledge.sources` (#1878/#1891); the discarded list names RAG sources the agent never recruits from. Compares by SET, so the same sources in a different order stay quiet. The three findings now share one builder, so they phrase the common half identically (which slot wins, that the alias is dropped, delete it) while each still names what its own discarded value would have done. No CLI change: both call sites already loop over whatever the pass returns. Also corrects `content/docs/ai/agents.mdx`, which documented `knowledge` as `{ topics, indexes }` and used `topics` in all three examples — teaching the deprecated alias as the canonical key, and contradicting `skills/objectstack-ai/SKILL.md`, which already had it right. Follow-ups filed: #3855 (removal schedule for all three aliases), #3856 (objectui's now-dead `execute || target` fallback).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #3754. Follows #3742 — same bug class, one field over, found while verifying that fix.
The shape
requiredWhenis canonical,conditionalRequiredis its documented deprecated alias. Unlikeaction.execute,FieldSchemahad no canonicalization at all — no lowering, no dropping. Both keys stayed live in the parsed output, so the precedence was re-implemented per consumer instead of being decided once in the contract. That is exactly the condition that produced #3713.And the alias surviving parse was test-pinned, including one case named literally:
— the inverse of the contract #3742 had just established one field over.
Honest framing: this is hardening, not a live-bug fix
Every reader I could inspect already prefers the canonical key:
packages/objectql/src/validation/rule-validator.tsrequiredWhen ?? conditionalRequired— canonical first ✅packages/lint/src/validate-expressions.tsSo no button is currently misbehaving. The problem is that nothing in the contract made the server right — the one surface most likely to have picked alias-first order is the same one that did in #3713, and it's the one I can't inspect from here. Folding once, in the schema, removes the chance rather than a defect. I'd rather state that plainly than dress this up as a bug fix.
What changed
FieldSchemagains a transform that lowersconditionalRequiredintorequiredWhenand drops the alias;requiredWhenwins when both are declared. Same shape asaction.execute→target(#3742) andagent.knowledge.topics→sources(#1891).objectql's fallback is deliberately kept.evaluateValidationRulesis also handed raw, unparsed field definitions — its own test suite does exactly that (rule-validator.test.ts, theconditionalRequiredcase) — so the alias-reading path still has real inputs. I commented it in place so it doesn't read as dead code, and noted that the order must stay canonical-first. This is the same defence-in-depth reasoning used for objectui in #3742, and it's why I did not follow step 3 of my own suggestion in #3754.Compatibility
Authoring is unchanged.
conditionalRequiredis 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:
parsedField.conditionalRequired→ TOparsedField.requiredWhenfield.conditionalRequired || field.requiredWhenbecomesfield.requiredWhenz.infer<typeof FieldSchema>no longer carries the alias, so a stale reader fails to compile rather than silently readingundefined.One naming note: I added
FieldParseInput(z.input<typeof FieldSchema>) for the author-facing shape.FieldInputwas already taken by an unrelated factory-helper type (Omit<Partial<Field>, 'type'>) further down the same file — worth knowing before reaching for that name.Risk that needed checking
This is the first transform on
FieldSchema, which turns it from aZodObjectinto a wrapped schema. Two things that would have broken:.shape/.extend/.partial/.omit/.pickdon't exist on a transform-wrapped schema. Swept the repo: no such use ofFieldSchema. ✅fieldproperty would go UNCLASSIFIED and the gate would fail. It unwraps generically (as it already must foraction). ✅ Gate passes.Verification
@objectstack/spec@objectstack/objectql@objectstack/runtime@objectstack/metadata@objectstack/lintpnpm buildcheck:livenesscheck:docsFollow-ups
field.conditionalRequiredandaction.execute). Now that neither reaches a renderer, that becomes a bounded change: drop the key, add a tombstone to the relevantUNKNOWN_KEY_GUIDANCEmap so the rejection carries the fix, and ship the migration note in the changeset (AGENTS.md Post-Task Checklist Implement ObjectStack protocol specification with Zod schemas and TypeScript interfaces #3).field.conditionalRequiredis the same alias-drift shape #3713 just fixed — and the spec pins the alias to SURVIVE parse #3754 still stands: the remaining@deprecated/alias declarations inpackages/spec/srcshould be audited — fold-and-drop ones are safe, merely-coexisting ones are latent #3713s.Generated by Claude Code