Reject unsupported async schemas in zod() and valibot()#701
Conversation
zod() now catches the raw Zod error from safeParse() when an async schema is used and re-throws a clear TypeError. valibot() now checks the schema's async property at construction time, since Valibot's safeParse() silently skips async validations instead of throwing. #462 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Address two review issues: - zod(): Match on Zod's $ZodAsyncError class name instead of regex on the error message, so unrelated errors containing "Promise" or "async" are not misreported as async-schema misuse. - valibot(): Recursively walk the schema tree to detect async parts inside wrappers like optional(), nullable(), nullish(), and union(). The top-level async property is false for these wrappers even when they contain async inner schemas. #462 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Address two remaining gaps:
- valibot(): Recurse into container members (object entries,
array item, tuple items, record/map key and value) and
pipeline schemas, not just wrappers and union options.
Previously, async schemas nested inside containers like
v.object({ a: v.pipeAsync(...) }) were silently accepted.
- zod(): Also match Zod v3's plain Error with message starting
with "Async refinement encountered during synchronous parse
operation", not just v4's $ZodAsyncError class, since the
package supports both Zod v3 (3.25+) and v4.
#462
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- valibot(): Traverse the rest field used by objectWithRest()
and tupleWithRest() in containsAsyncSchema().
- zod(): Also recognize Zod v3's async transform error message
("Asynchronous transform encountered during synchronous parse
operation"), not just the refinement-specific one.
#462
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- valibot(): Invoke the getter of v.lazy() schemas to check whether the inner schema contains async parts. Previously, lazy() kept async === false on the outer layer, bypassing the guard entirely. - zod(): After safeParse() returns a failure, check for invalid_type issues with received: "promise", which is how Zod v3 reports async preprocessors instead of throwing. This complements the existing catch-based detection. #462 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- valibot(): Descend into v.promise() inner schemas, which store the resolved-value schema in an overloaded message field rather than the usual wrapped field. - zod(): Recurse into unionErrors inside invalid_union issues when scanning for promise-type mismatches, so async preprocessors inside z.union() are correctly rejected on Zod v3. #462 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Stop recursing into union/variant options when scanning for async schemas. CLI parsers always supply string input, so synchronous arms that match the string short-circuit before any async arm is reached. Rejecting unions just because an unreachable arm is async would break reusable schemas that work fine for CLI parsing. #462 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- intersect: always recurse into options, since all arms must
match and async arms are always reachable
- union/variant: only skip options recursion when a bare
v.string() arm (no pipe, sync) exists as a catch-all for
CLI string input; otherwise recurse, since async arms can
be reached when no sync arm covers all inputs
This fixes two gaps: intersect with async arms silently
skipping validation, and unions like
v.union([v.literal("a"), asyncSchema]) crashing at parse time
for non-matching inputs.
#462
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- zod(): Require code === "invalid_type" when checking for received: "promise" issues, so that literal schemas parsing the string "promise" are not misclassified as async errors. - valibot(): Track whether the schema is after a v.transform() in a pipeline. The union catch-all string arm optimization is only valid for raw CLI string input; after a transform the input type is unknown, so all union arms must be checked. - valibot(): Recognize optional/nullable/nullish-wrapped v.string() as catch-all union arms, since the unwrapped string schema still accepts every CLI string. - valibot(): Extract isCatchAllStringSchema() helper that unwraps optional/nullable/nullish wrappers before checking for a bare v.string(). #462 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Remove v.lazy() getter probing: the getter receives actual parse input, so calling it with no argument at construction time takes unpredictable branches and misses input-dependent async schemas. Document as a known limitation. - Remove afterTransform tracking: we cannot inspect transform return types at runtime, so string-preserving transforms like s.trim() were being incorrectly flagged. The union catch-all string arm heuristic now applies uniformly. #462 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- isCatchAllStringSchema(): Accept any wrapper with a wrapped field (nonOptional, exactOptional, etc.), and recognize piped string schemas where all pipe actions are non-validation (transforms, trim, toLowerCase, etc. never reject input). - Restore afterTransform tracking: after a v.transform() in a pipeline, the union catch-all optimization is disabled since the output type is unknown. Valibot's built-in actions like v.trim() are NOT transforms and are correctly recognized as non-rejecting in isCatchAllStringSchema(). #462 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Propagate afterTransform = true into container members (entries, item, items, key, value, rest, promise), since nested schemas inside containers no longer see the raw CLI string input. This catches async branches inside object/ array/tuple members that follow a type-changing transform. - Rename isCatchAllStringSchema to isCatchAllSchema and add v.unknown() and v.any() as recognized catch-all types, since these synchronously accept every value and short-circuit before async arms in unions. #462 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Remove piped string schemas from catch-all detection: pipes with transforms can change the value type, and subsequent schemas in the pipe can reject, so the arm may not actually accept all inputs. Only bare v.string() without pipe counts. - Split catch-all logic by context: v.unknown()/v.any() are catch-all regardless of transforms (they accept any type), but bare v.string() only counts before transforms (when input is still known to be a string). - Add includeString parameter to isCatchAllSchema() so the union check can disable string catch-alls after transforms while still recognizing type-agnostic catch-alls. #462 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Piped v.unknown()/v.any() schemas can have later pipe steps that reject, so v.pipe(v.unknown(), v.string()) is not a true catch-all. Only bare schemas without a pipe are recognized. #462 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The afterTransform flag cannot reliably distinguish string-preserving transforms from type-changing ones, causing false positives on valid parsers like v.pipe(v.string(), v.transform(s => s.trim()), v.union(...)). Remove the flag entirely and simplify: a bare v.string(), v.unknown(), or v.any() arm in a union always short-circuits the catch-all optimization. Type-changing transforms before unions with a v.string() catch-all arm are a known limitation documented in the JSDoc. Also remove the includeString parameter from isCatchAllSchema and the afterTransform propagation into container members, which are no longer needed. #462 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Piped schemas like v.pipe(v.string(), v.trim()) are catch-all when every pipe action is a non-rejecting transformation (not a validation or nested schema). This avoids false positives on unions that normalize string input before branching. #462 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- valibot(): Best-effort check for v.lazy() by calling the getter with no arguments. Constant getters (the common case) return the inner schema for async inspection; input- dependent getters may throw, which is safely ignored. - zod(): Match exact Zod v3 error messages instead of prefixes, so a user transform that throws an Error starting with "Async refinement encountered..." is not misclassified as an async-schema error. #462 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Use a WeakSet to track visited schemas during the async detection walk. This prevents recursive v.lazy() schemas (e.g., tree nodes) from causing repeated getter evaluations and eventual stack overflow during parser construction. #462 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
v.variant() stores its discriminator in the key field as a plain string, not a schema object. Add a typeof === "object" guard before recursing into s.key to avoid crashing with "Invalid value used in weak set" on variant schemas. #462 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- isCatchAllSchema(): Skip the first pipe element (the base schema with kind === "schema") when checking for rejecting actions. The base schema is always the first item in the pipe array; only subsequent actions need to be non-rejecting. This fixes v.pipe(v.string(), v.trim()) being incorrectly rejected as a union arm. - Remove v.lazy() getter probing entirely. Executing the getter during construction changes observable behavior for stateful or input-dependent getters. Lazy schemas are now a documented known limitation of the async detection. #462 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
A synchronous z.preprocess() that returns a Promise object produces the same invalid_type/received:"promise" issue as an async preprocess, making them indistinguishable from parse results alone. Remove the result-based check to avoid misclassifying sync schemas as async. Zod v3 async preprocessors that don't throw are now a known limitation. The catch-based detection still handles direct async refinements/transforms on both Zod v3 and v4. #462 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
CLI input is always a string, so container schemas (object,
array, tuple, etc.) reject it at their own type check before
visiting members. Only check container members when the
container is reachable — i.e., inside a pipeline where a
transform may have changed the value type.
Add a checkContainers parameter (default false) that pipeline
schema action recursion sets to true. This prevents
valibot(v.object({ a: asyncSchema })) from being rejected at
construction time when the async entry is unreachable.
#462
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Use a WeakMap<object, boolean> instead of WeakSet to record whether each schema was visited with checkContainers enabled. A shallow visit (checkContainers=false) no longer prevents a later deep visit (checkContainers=true) of the same reused schema instance, so async members inside piped containers are still detected even when the same schema was previously seen in an unreachable top-level context. #462 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- isCatchAllSchema(): Also exclude raw_transform pipe actions, since v.rawTransform() can add issues (reject input) unlike built-in transformations like trim() or toLowerCase(). - containsAsyncSchema(): Scan union/variant options left-to- right, matching Valibot's evaluation order. Only stop at a catch-all arm; async arms before the catch-all are still detected. This fixes v.union([asyncArm, v.string()]) being incorrectly accepted when the async arm is tried first. #462 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Before any transform in a pipeline, the input is still a
string and container schemas (object, array, etc.) will reject
it at their own type check. Only enable checkContainers for
pipe schema actions that appear after a transform or
rawTransform action.
This fixes v.pipe(v.string(), v.object({ a: asyncInner }))
being incorrectly rejected when the async entries are
unreachable from string input.
#462
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
After processing a pipe schema action, check if that schema's
own pipe contains transform or rawTransform actions. If so,
mark seenTransform = true for subsequent steps in the outer
pipe, so container members after a nested type-changing
transform are correctly treated as reachable.
This fixes v.pipe(v.string(), v.pipe(v.string(),
v.transform(JSON.parse)), v.object({ a: asyncInner })) being
incorrectly accepted when the nested pipe changes the type.
#462
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Container member checking (entries, items, key, value, rest, promise inner) cannot be done correctly without type tracking through transforms. Type-preserving transforms (s.trim()) and type-changing transforms (JSON.parse) are indistinguishable at construction time, causing either false positives (rejecting valid sync schemas) or false negatives (missing async members after type-changing transforms). Remove container checks entirely. At the top level, CLI input is always a string, so container schemas reject before visiting members. Async schemas inside containers after transforms are documented as a known limitation. Also removes pipeContainsTransform, checkContainers parameter, seenTransform tracking, and WeakMap-based visited state (simplified back to WeakSet since container depth tracking is no longer needed). #462 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
After a v.transform() or v.rawTransform() in a pipeline, the
value type is unknown, so:
- Container members (entries, items, key, value, rest) become
reachable and must be checked for async schemas.
- String-based catch-all arms in unions are no longer trusted;
only type-agnostic catch-alls (v.unknown(), v.any()) suppress
async checking after transforms.
This catches cases like v.pipe(v.string(), v.transform(
JSON.parse), v.object({ a: asyncInner })) where the transform
changes the value type and async entries become reachable.
Type-preserving transforms (e.g., s => s.trim()) before
containers or unions with string catch-alls will be treated
conservatively as potentially type-changing — a deliberate
trade-off to prevent silent async validation skipping.
#462
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5d466a5153
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/valibot/src/index.ts`:
- Around line 128-130: Update the `v.lazy()` limitation comment to reflect that
the code now performs a best-effort getter probe for lazy schemas (the probe
implemented near the v.lazy() inspection logic) rather than declaring them
entirely uninspected; state the current behavior (the probe calls the lazy
getter with a synthetic probe value to try to derive schema info) and list the
remaining blind spots to be explicit: getters that rely on actual parse input,
getters with side effects or external state, getters that throw or return
promises, and any runtime-dependent behavior that can't be statically inferred.
Keep the note brief and replace the old blanket "not inspected" statement with
this updated description referencing v.lazy() and the getter probe.
- Around line 237-243: The code calls schema.getter via unsafe casts and assumes
it always returns a schema, which can pass null/undefined into
containsAsyncSchema and trigger a WeakMap TypeError; update the
ValibotSchemaInternal type to include an optional getter signature (e.g.,
getter?: () => v.BaseSchema<...>), remove the casts like "as unknown as {
getter?: ... }" and access schema.getter in a type-safe way, then guard the
getter's return (e.g., const inner = schema.getter?.(); if (!inner)
continue/return false) before calling containsAsyncSchema(inner, visited,
afterTransform); ensure you only call containsAsyncSchema when inner is a valid
schema object.
- Around line 164-170: The reachability check currently treats s.type ===
"variant" like "union" but isCatchAllSchema() doesn't recognize object-shaped
variant arms, so broad variant discriminators don't short-circuit and later
async arms cause false positives; to fix, split the branch for isUnionLike into
separate handlers: keep the existing union path using isCatchAllSchema(option,
afterTransform) and containsAsyncSchema, and add a variant path that treats each
option as an object arm—implement a new helper (e.g., isVariantCatchAll or
extend isCatchAllSchema to handle object arm schemas) to detect a catch-all
discriminator on object arms and short-circuit accordingly before calling
containsAsyncSchema on later arms; update references in the main loop (the
s.options iteration and isUnionLike handling) and add tests for variant([{type:
"default", ...}, asyncArm]) to cover async arms after a broad discriminator.
🪄 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: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 7a5eac72-1049-40c8-899b-484c905b1cbd
📒 Files selected for processing (2)
packages/valibot/src/index.test.tspackages/valibot/src/index.ts
- zod(): Add "Synchronous parse encountered promise." to the list of exact Zod v3 error messages recognized by isZodAsyncError(), covering async schemas nested under parent parsers like z.preprocess(..., z.object(...)). - valibot(): Remove v.lazy() getter probing. The getter receives actual parse input and may return different schemas depending on it (e.g., sync for strings, async for other types). Probing with no argument takes the wrong branch for input-dependent getters, causing false positives and potentially triggering user side effects. #462 #701 (comment) #701 (comment) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
@codex review |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/valibot/src/index.ts`:
- Around line 121-135: Update the containsAsyncSchema() JSDoc to explicitly
document the v.variant() limitation: note that containsAsyncSchema() treats
v.variant() like v.union() for left-to-right arm reachability, but
isCatchAllSchema() only recognizes bare string/unknown/any (and their wrapped
forms) and does not detect object-shaped discriminator arms, which can cause
conservative false positives when a variant has an async arm that may be
unreachable; reference isCatchAllSchema(), containsAsyncSchema(), v.variant(),
and v.union() in the comment so readers know where the limitation originates.
🪄 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: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 281b9699-f7ce-4b6c-a7c3-c58705a9240c
📒 Files selected for processing (3)
packages/valibot/src/index.test.tspackages/valibot/src/index.tspackages/zod/src/index.ts
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c6caf39c0d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Replace blocklist-based transformation checks with an allowlist (SAFE_TRANSFORMATION_TYPES) of known non-rejecting, type-preserving Valibot actions: trim, to_lower_case, to_upper_case, normalize, to_min_value, to_max_value. All other transformation actions (transform, raw_transform, parse_json, to_number, to_boolean, etc.) are now treated as potentially type-changing, both for: - seenTransform tracking (enables container member checks) - isCatchAllSchema pipe checks (excludes from catch-all) This fixes v.parseJson() and similar built-in transformations being incorrectly treated as safe in union catch-all detection and not triggering afterTransform for container checks. Also documents v.variant() catch-all limitation in JSDoc. #462 #701 (comment) #701 (comment) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/valibot/src/index.ts`:
- Around line 101-118: isCatchAllSchema incorrectly treats piped
v.any()/v.unknown() as catch-alls when afterTransform=true; change
isCatchAllSchema so that if s.type is "unknown" or "any" and afterTransform is
true and s.pipe exists, it immediately returns false (i.e., piped arms are not
trusted post-transform). Update isCatchAllSchema (and any callers expecting this
behavior such as containsAsyncSchema/valibot constructor checks) to use this
early-return, and add the regression test: valibot() should reject
v.pipe(v.string(), v.transform(JSON.parse), v.union([v.pipe(v.any(), v.trim()),
asyncArm])).
🪄 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: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: f3bfb25c-4b68-423a-a5c8-7346df608340
📒 Files selected for processing (1)
packages/valibot/src/index.ts
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ea6b1d1b76
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
- Add trim_start and trim_end to SAFE_TRANSFORMATION_TYPES allowlist. These are non-rejecting, type-preserving string transformations like trim. - Recognize v.fallback() schemas as catch-all union arms. Fallback schemas always succeed (they return the fallback value when validation fails), so later async arms in a union are unreachable. #462 #701 (comment) #701 (comment) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9095eff27d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
- Add readonly, brand, flavor to SAFE_TRANSFORMATION_TYPES. These are type-level metadata actions that never reject or change values at runtime. - Check v.promise() inner schema (stored in .message field) in the afterTransform container block, so async schemas inside promise wrappers after transforms are detected. #462 #701 (comment) #701 (comment) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5e908f40e7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
- isCatchAllSchema(): Instead of rejecting kind === "schema"
pipe actions outright, recursively check if the nested schema
is itself a catch-all. This fixes v.pipe(v.string(),
v.pipe(v.string(), v.trim())) being rejected as a union arm.
- containsAsyncSchema(): After processing a schema-typed pipe
action, inspect its inner pipe for non-safe transforms. This
fixes v.pipe(v.string(), v.pipe(v.string(),
v.transform(JSON.parse)), v.object({ a: asyncInner })) not
being detected because the outer pipe never saw a transform.
#462
#701 (comment)
#701 (comment)
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 531ac817b4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
- isCatchAllSchema(): Use "fallback" in s instead of s.fallback !== undefined, so v.fallback(schema, undefined) is correctly recognized as catch-all. - containsAsyncSchema(): Split variant handling from union. Variant schemas only parse objects, so at top level (before transform) string input fails the outer type check and arms are unreachable. Only inspect variant arms after a transform. #462 #701 (comment) #701 (comment) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4fb53937b6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Piped wrapper schemas (e.g., v.pipe(v.optional(v.string()), v.transform(...))) have both wrapped and pipe fields. Previously, the early return via s.wrapped skipped the pipe inspection entirely, missing reachable async branches after transforms and incorrectly treating piped wrappers with rejecting actions as catch-all union arms. Now both isCatchAllSchema() and containsAsyncSchema() only unwrap via s.wrapped when there is no pipe. #462 #701 (comment) #701 (comment) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a30cf45c9f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Summary
When an async Zod schema (e.g., one with async refinements) was passed to
zod(), callingschema.safeParse(input)threw a raw Zod library error instead of a clear Optique-level error. Similarly,valibot()silently skipped async validations by returning{ success: true }withundefinedoutput when given an async schema throughsafeParse(). This change makes both wrappers detect unsupported async schemas and throw a clearTypeErrorat construction time or parse time, depending on the adapter.Fixes #462
@optique/zodThe
zod()wrapper now catches errors thrown byschema.safeParse(input)and checks whether they indicate an async schema. On Zod v4, the dedicated$ZodAsyncErrorclass is recognized. On Zod v3 (3.25+), the two exact error messages for async refinements and async transforms are matched:Unrelated errors from user code inside
refine()/transform()are not intercepted. The@throwsJSDoc onzod()has been updated to document the newTypeError.@optique/valibotThe
valibot()wrapper performs a construction-time schema walk (containsAsyncSchema()in packages/valibot/src/index.ts) that detects async parts in common schema structures. When async schemas are found, aTypeErroris thrown immediately rather than allowingsafeParse()to silently skip async validations at parse time:The detection covers:
schema.async === true)v.string(),v.unknown(),v.any(), or piped variants with only non-rejecting actions) stops the scan since later arms are unreachableasyncflags or nested schema-typed actionsv.transform()/v.rawTransform()in a pipeline, container members (object entries, array items, tuple items, record key/value, rest schemas) become reachable and are recursively checked; string-based catch-all arms in unions are no longer trusted since the value type may have changed (onlyv.unknown()/v.any()are recognized as type-agnostic catch-alls after transforms)Known limitations documented in the JSDoc:
v.lazy()schemas are not inspected because the getter depends on actual parse input, making static analysis unreliables => s.trim()viav.transform()) are treated conservatively as potentially type-changing, since the output type cannot be determined at construction time; users should use Valibot's built-in actions likev.trim()(which are recognized as non-rejecting) orv.unknown()/v.any()catch-all arms when composing unions after transformsTest plan
mise testpasses across all runtimes (Deno, Node.js, Bun)zod()throwsTypeErrorfor async refinements and async transformszod()does not intercept unrelated user errors fromtransform()valibot()throwsTypeErrorfor direct async schemas, wrapped async, union without catch-all, intersect with asyncvalibot()allows unions with catch-all arms (barev.string(),v.unknown(), pipedv.pipe(v.string(), v.trim()))valibot()rejects async entries/union arms afterv.transform()in a pipelinevalibot()allows direct containers (object, array, tuple) with async members when not inside a pipeline (string input rejects at the container type check)