fix(zod): keep required on constraint-only oneOf/anyOf branches - #3783
Merged
melloware merged 2 commits intoJul 29, 2026
Merged
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughZod generation now applies ChangesZod union constraint handling
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related issues
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/zod/src/index.ts`:
- Around line 303-317: Update isConstraintOnlyMember to reject schemas
containing additionalProperties, so members are classified as constraint-only
only when they contain required constraints without additional schema behavior.
Preserve the existing handling for other schema keywords and add a regression
case proving additionalProperties reaches the normal generator path instead of
being discarded.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 136d7cfe-959e-43ef-81fd-5ef1cb550922
📒 Files selected for processing (2)
packages/zod/src/index.tspackages/zod/src/zod.test.ts
ostapondo
force-pushed
the
fix/zod-constraint-only-oneof-branches
branch
from
July 29, 2026 12:52
9a1f0f7 to
b014b00
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/zod/src/zod.test.ts`:
- Around line 12212-12224: The regression assertions are too weak to detect the
reported behavior. In packages/zod/src/zod.test.ts lines 12212-12224, update the
test around “leaves branches that declare their own shape untouched” to match
the actual integer output, zod.number().int(), and assert the shaped branch
contains only its own properties. In packages/zod/src/zod.test.ts lines
12281-12289, strengthen the allOf test to require A to be non-optional and,
ideally, verify B remains optional.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: aa9c7b8c-8c57-49ea-8c09-5670afdf8f99
📒 Files selected for processing (2)
packages/zod/src/index.tspackages/zod/src/zod.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/zod/src/index.ts
@orval/angular
@orval/axios
@orval/core
@orval/effect
@orval/fetch
@orval/hono
@orval/mcp
@orval/mock
orval
@orval/query
@orval/solid-start
@orval/swr
@orval/zod
commit: |
ostapondo
force-pushed
the
fix/zod-constraint-only-oneof-branches
branch
from
July 29, 2026 13:08
b014b00 to
327bbe4
Compare
ostapondo
force-pushed
the
fix/zod-constraint-only-oneof-branches
branch
from
July 29, 2026 13:49
327bbe4 to
75d7ffd
Compare
A branch that declares no shape of its own and only lists `required` was rendered in isolation, where it has no type to resolve and falls through to `zod.unknown()`. The constraint was silently dropped, so the generated validator accepted payloads the spec rejects. JSON Schema applies every branch to the same instance, so the property types live on the composing schema. Render such branches against those sibling properties instead, which is what the `allOf` path already achieves through `additionalRequired`. A member only counts as shape-less when `required` is all it says, aside from `title`/`description` (annotations, carried over) and `not` (not translated by this generator today). Members carrying `enum`, `const`, `additionalProperties`, `nullable` or `default` already render to something meaningful and are left untouched.
ostapondo
force-pushed
the
fix/zod-constraint-only-oneof-branches
branch
from
July 29, 2026 13:51
75d7ffd to
1053abc
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/zod/src/zod.test.ts (1)
12229-12238: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winCover
defaultas a standalone branch constraint.
defaultis intentionally excluded from constraint-only rewriting, but this matrix does not test it. Add a case that verifies the default-bearing member remains standalone rather than receiving sibling properties.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/zod/src/zod.test.ts` around lines 12229 - 12238, Add a `default` case to the `it.each` matrix for “leaves a member carrying %s alone,” using a representative default constraint and expected output that confirms the member remains standalone without sibling properties. Keep the existing constraint-only cases unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/zod/src/zod.test.ts`:
- Around line 12187-12199: Strengthen the requiredness assertions in the three
test cases: in the branch test at packages/zod/src/zod.test.ts lines
12187-12199, assert Y is present and optional in the AB branch and required in
the XY branch; at lines 12201-12210, assert the complete A/B and X/Y
requiredness for both anyOf branches; and at lines 12249-12271, assert the
not-bearing branch requires A and B rather than only checking it is not
zod.unknown().
---
Nitpick comments:
In `@packages/zod/src/zod.test.ts`:
- Around line 12229-12238: Add a `default` case to the `it.each` matrix for
“leaves a member carrying %s alone,” using a representative default constraint
and expected output that confirms the member remains standalone without sibling
properties. Keep the existing constraint-only cases unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 53784afc-96c3-4c97-8489-cc44f9a39070
📒 Files selected for processing (2)
packages/zod/src/index.tspackages/zod/src/zod.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/zod/src/index.ts
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/zod/src/index.ts`:
- Around line 792-798: Update the synthesized OpenApiSchemaObject branch in the
relevant schema conversion logic to retain every key listed in required,
including keys absent from schema.properties; add those missing keys with the
applicable additional-properties schema where available so an empty object
cannot match. Preserve the existing description and declared-property handling,
and add a regression test for required: ['kind'] without properties.kind.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 235980ca-2e89-47a9-ab80-155b52b7f3a0
📒 Files selected for processing (2)
packages/zod/src/index.tspackages/zod/src/zod.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/zod/src/zod.test.ts
A branch may require a key the composing schema never declares. Rewriting it produced an object that omitted the key entirely, so an empty payload matched a branch that should have rejected it. zod cannot express `present, type unspecified` — `unknown` and `any` are both optional inside an object — so such a branch now keeps the existing behaviour instead of an object that only looks like it enforces the constraint. Also pin both branch bodies whole in the regression tests; the previous assertions stopped short of the last property and would have passed had it gone missing.
Contributor
Author
|
@coderabbitai review |
Contributor
✅ Action performedReview finished.
|
melloware
approved these changes
Jul 29, 2026
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.
Addresses the
zod.unknown()half of #3780.A
oneOf/anyOfbranch is allowed to declare no shape of its own and only listrequired, leaving the property types on the composing schema — the two branchesin #3780 are exactly that. Each branch is rendered on its own, where there is no
typeto resolve, so it falls through tozod.unknown()and itsrequiredisdropped. The result is
zod.union([zod.unknown(), zod.unknown()]).and(...), whichaccepts everything the union was supposed to constrain.
Since JSON Schema applies every branch to the same instance, those branches are
now rendered against the composing schema's
properties. TheallOfpath alreadygets the same effect via
additionalRequired(#3171); this is theoneOf/anyOfcounterpart. Branches that describe a shape of their own are untouched.
A member only counts as shape-less when
requiredis all it says, aside fromtitle/description(annotations, carried over so.describe()survives) andnot, which this generator does not translate into anything today. Memberscarrying
enum,const,additionalProperties,nullableordefaultalreadyrender to something meaningful and are left untouched — I compared every such
keyword against
masterto make sure nothing else changes.Against the repro spec,
emptyandincompleteXYnow fail as they should, andthe two valid payloads still pass.
invalidMixedstill passes, because that oneneeds the
notin the AB branch — orval has nonotsupport anywhere today, sothat felt like a separate change rather than something to fold in here.
tests/specifications/one-of-required.yamlalready has this schema shape, but itis only wired into
default.config.ts, so no generated output changes. I checkedthe other 349 specs in the repo for the pattern; none are affected. Verified end to end by
running the CLI over the spec from the issue and parsing the five payloads with
zod: 4 of 5 now match, against 2 of 5 before. Regression tests added, full suite
and typecheck pass.
Summary by CodeRabbit
oneOf/anyOfbranches that specify onlyrequiredconstraints, applying them to sibling properties instead of producing fallbackunknownbranches.descriptionand supports branches that includenot, while treating$refbranches as non-constraint-only.allOfbehavior unchanged.oneOf/anyOfcases, including required/description handling, sibling-absence behavior, and constraint-carrying members within branches.