fix: preserve schema helper types#1981
Conversation
There was a problem hiding this comment.
LGTM. Follow-ups noted below. The collapse pass fires at the right layer — generator post-processing, not adopter code — and only when union arms are provably opaque (z.record(z.string(), z.unknown()) recursively, including named refs like SizeModeMutexSchema). Runtime validation set is unchanged because union([opaqueRecord, ...]).and(realObject) ≡ realObject; the win is purely type-side helper restoration.
Things I checked
- Runtime equivalence on all four collapsed schemas.
V1ProductNamedFormatReferenceSchema,V2ProductInlineFormatDeclarationsSchema,FixedSchema,MultiSizeSchema,ResponsiveSchema,NoneSchema, and theSizeModeMutexSchemathey feed all resolve toz.record(z.string(), z.unknown())in the current generated file. Dropping them from the runtime expression is a true no-op on validation. The "stops firing when markers gain real fields" test attest/generate-zod-object-intersections.test.js:101-110is the load-bearing guard — once V1/V2 grow real fields, the collapse stops and maintainers have to think. - Cycle / depth correctness in the new pass.
isOpaqueRecordMarkerExpressionandisOpaqueMarkerUnioneach carry avisitingset (scripts/generate-zod-from-ts.ts:185, 211);splitTopLevelListtracks()/{}/[]depth and skips quoted/regex literals;rewriteLeadingMarkerUnionObjectAndonly rewrites when the tail is exactly.and(<object-shape>)at depth 0, so discriminated-union response intersections likeCreateMediaBuyResponseSchemaare preserved. - Pass ordering.
postProcessForPassthrough→postProcessMarkerUnionObjectIntersections→postProcessObjectIntersections→ TS7056 annotations (scripts/generate-zod-from-ts.ts:1199-1276). Marker collapse runs before object-intersection merge so the latter can pick up the now-flat object form. - Typed dictionary coverage.
KnownToolRequestSchemas(src/lib/utils/tool-request-schemas.ts:33-89) enumerates every key currently inTOOL_REQUEST_SCHEMAS. The fallback[toolName: string]: z.ZodObject<any> | undefinedmakes the dynamic cast atsrc/lib/server/create-adcp-server.ts:3455legal withoutas unknown as.WithOptionalAccountShape<TShape>correctly preserves per-field types after theaccountrequired→optional flip. - TS7056 annotation shape. Switching
PreviewCreativeRequestSchemaandUpdateMediaBuyRequestSchematoobjectShape: trueemits the dualz.ZodObject<{ [K in keyof T]-?: z.ZodType<T[K], T[K]> }, any> & z.ZodType<T & Record<string, unknown>, ...>annotation (schemas.generated.ts:6073, 9003) —.shapeaccess plusRecord<string, unknown>widening, both preserved. - Changeset adequacy for wire impact.
patchis right for the runtime-visible change. Type narrowing on the exported maps is strictly additive againstPartial<Record<string, z.ZodType>>— old read-side uses still type-check becauseZodObject extends ZodType.
Follow-ups (non-blocking — file as issues)
- Changeset prose understates the typed-map narrowing.
.changeset/product-schema-object-helpers.mdonly mentionsProductSchemaand canonical formats. Adopters who explicitly assigned the map toRecord<string, z.ZodObject<any>>(noPartial, no| undefined) will see a TS error from the new readonly index signature. One-line fix on their side, but worth a sentence in the changelog so they're not surprised. - JSDoc on
KnownToolRequestSchemasand on the narrowing behavior ofTOOL_INPUT_SHAPES[toolName]. Adopters will currently discover this by IDE autocomplete. One sentence on each closes the loop. - Don't-replicate hint on the TS7056 annotation. The verbose dual annotation in
schemas.generated.ts:6073, 9003is what an LLM sees in a hover and may try to mimic when authoring a new schema. The comment atscripts/generate-zod-from-ts.ts:102-112explains why it exists; a single sentence pointing atTS7056_SCHEMASas the opt-in mechanism would head off the cargo-culting.
Minor nits (non-blocking)
- Redundant
update_rightsline.src/lib/schemas/index.ts:51—update_rights: typeof schemas.UpdateRightsRequestSchema.shapeis already provided by the mapped half at line 48 (update_rightsis aKnownToolRequestSchemaskey). Harmless intersection of the same type with itself. Onlycreative_approvalgenuinely needs the additional intersection. Drop the line. - Type-test could
@ts-expect-errorpastvoid.src/type-tests/zod-object-intersections.type-test.ts:21-29—productPick,productOmit,productExtendarevoid-referenced. Asserting on a field-access of those (productPick.shape.product_id) would more directly prove the helpers return ZodObjects rather than ZodIntersections. Non-blocking — the existing assertions already fail at compile time if the helpers disappear.
Notable choice: pinning the collapse to opaque-record arms specifically (rather than any union arm with no real schema) means the pass intentionally won't fire on the day V1/V2 grow real validators. That's the right shape — fail-loud at the seam, don't translate.
Approving.
757b376 to
314c1cc
Compare
There was a problem hiding this comment.
LGTM. Follow-ups noted below. Restoring .shape/.extend/.omit/.pick on ProductSchema and the canonical-format schemas without changing what the SDK accepts on the wire is the right shape — both arms of the collapsed unions resolve to z.record(z.string(), z.unknown()), so union ∩ object.passthrough() and object.passthrough() accept the identical value set. Witness, not translator, intact.
Things I checked
- Wire-shape invariant.
V1ProductNamedFormatReferenceSchema,V2ProductInlineFormatDeclarationsSchema, and all fourSizeModeMutexarms (Fixed / MultiSize / Responsive / None) are literalz.record(z.string(), z.unknown())insrc/lib/types/schemas.generated.ts. They carry no fields and impose no validation beyond "is an object" — which.passthrough()on the object half already enforces.safeParsereturns identical results before and after. - Spec conformance. The V1/V2 product split is an at-least-one-of presence constraint on
format_ids/format_options(both optional on the combinedProduct, coexisting via dual emission per the 7.9→7.10 notes), not a discriminated union. Same for size-mode mutex — spec prose atsrc/lib/types/core.generated.ts:3665, not a structural enforcer. SDK was never enforcing either discriminator; this PR doesn't change that. - Safety net.
isOpaqueMarkerUnion(scripts/generate-zod-from-ts.ts:876-901) requiresarms.every(...)to be opaque records — unanimity, not majority. If a future spec rev materializes a real field on V2 product, the arm stops being opaque,.everyfails, the intersection is preserved verbatim. Fail-loud on regression. - Ordering.
postProcessMarkerUnionObjectIntersectionsruns beforepostProcessObjectIntersections(scripts/generate-zod-from-ts.ts:1209-1216). Marker pass strips the.union(...).and(...)wrapper, leaving a bare object; the object-intersection pass then sees no top-level.and()for these schemas. No undo. - MCP runtime contract. The new TS7056 annotation
z.ZodObject<{[K in keyof T]-?: z.ZodType<T[K], T[K]>}, any> & z.ZodType<T & Record<string, unknown>, ...>is type-only — RHS is still a plainz.object({...}).passthrough()call.TOOL_REQUEST_SCHEMAS.preview_creative.shapereturns the realZodRawShapethatserver.registerToolexpects. Noas anysmuggling. - Known vs. fallback resolution.
Readonly<KnownToolRequestSchemas> & { readonly [toolName: string]: z.ZodObject<any> | undefined }resolves known keys to exact types before the index fallback. The@ts-expect-errorassertions atsrc/type-tests/zod-object-intersections.type-test.ts:47, 54, 58, 105lock this. - Changeset.
patchis correct — no runtime behavior change, no public API removed, helper access restored.
Follow-ups (non-blocking — file as issues)
KnownToolRequestSchemasdrift is silent. The intersection atsrc/lib/utils/tool-request-schemas.ts:99-101accepts any new runtime entry via the string index. A tool added toTOOL_REQUEST_SCHEMASbut missed inKnownToolRequestSchemaspassestscand silently widens toz.ZodObject<any> | undefinedat the call site — adopters lose.shape.<field>narrowing with no error. Add asatisfiescheck or an exhaustivekeyofassertion (e.g., a never-typed_exhaustive: keyof KnownToolRequestSchemas extends keyof typeof TOOL_REQUEST_SCHEMAS ? ... : never) so drift fails the build. Theci:schema-checkreference atsrc/lib/schemas/index.ts:74covers map presence, not type-map membership.- Opaque-marker detection is a literal string compare.
scripts/generate-zod-from-ts.ts:859matches exactlyz.record(z.string(), z.unknown()). One ts-to-zod output tweak (whitespace, member order) silently disables the collapse, helpers regress, and nothing fails —.parse()still works. Worth a post-generation invariant: assertProductSchema._def.typeName === 'ZodObject'(ortypeof ProductSchema.shape === 'object') at build time so a regression fires loudly. - Pre-existing size-mode
oneOfgap. The SDK has never enforced the Fixed/MultiSize/Responsive/None mutex on canonical formats; that lives in spec prose. This PR makes the gap more visible (no longer hidden behind a meaningless.and()) but doesn't widen it. If the working group wants SDK-side enforcement, that's separate codegen work — materialize the size-mode discriminator into realz.objectarms with required-field disjointness.
Minor nits (non-blocking)
ZodObject<..., any>policy slot.scripts/generate-zod-from-ts.ts:118— theanysecond-type-param is the unknown-keys-policy slot. The schema is.passthrough(), but'passthrough'isn't part of Zod's public type surface, soanyis the only sound value. One-line comment on this would save the next reader a Zod source spelunk.splitTopLevelListbias.scripts/generate-zod-from-ts.ts:822-833— returningundefinedon an empty part means "trailing comma → not a marker union → leave intact." Conservative-correct but worth a one-line comment so a future maintainer doesn't try to be helpful.- Fallback shape assertion in type-test.
src/type-tests/zod-object-intersections.type-test.ts:95-99exercises the string-index fallback but doesn't assert the inferred type. Aconst _expected: z.ZodObject<any> | undefined = runtimeRequestSchema;would lock the fallback shape so accidental widening of known keys to the fallback breaks the test.
Safe to merge.
* fix: preserve ProductSchema ZodObject ergonomics * chore: sync package lock versions * fix: unwrap named record union schema markers * chore: add ProductSchema DX changeset * fix: identifier-boundary guard + loud-fail in named-union unwrap Address two reviewer follow-ups on #1972 (`code-reviewer` and `javascript-protocol-expert` converged on the same nit independently): 1. Left identifier-boundary check in `unwrapNamedRecordUnionIntersections`. `content.startsWith(${name}.and(, i)` previously matched suffixes — `SizeModeMutexSchema` collected today, but a future `FooSizeModeMutexSchema` would silently corrupt output by emitting the prefix character and then the inner schema body. Gate on `i === 0 || !/[A-Za-z0-9_$]/.test(content[i-1])`. 2. Loud-fail when `readBalancedBody` returns undefined. Previously swallowed silently (advanced by name length, dropping content); now throws so a malformed ts-to-zod payload crashes codegen. Also extends `zod-schema-ergonomics.type-test.ts` with `.extend()` / `.omit()` / `.pick()` assertions against `CanonicalFormatDisplayTagSchema`, `CanonicalFormatImageSchema`, and `CanonicalFormatHTML5BannerSchema` — the three Pass 4 targets the original PR only covered via runtime `.d.ts` grep. Regen brings inline-enums and schemas.generated forward. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(codegen): fold #1979 hardening items before merge - Pair negative .d.ts intersection grep with runtime _def.typeName===ZodObject assertion in zod-schemas.test.js so the test fails closed if Zod adjusts its tuple stringification. - Note the z.object( body-prefix constraint in both unwrap docstrings so a future editor doesn't accidentally broaden the right-hand side. - Add TODO comment on the one-level limit in collectRedundantRecordUnionSchemaNames. Refs #1979 https://claude.ai/code/session_01768uE6DmJzUwwq4tpb4hqZ * fix(test): use ZodObject constructor name, not Zod 3 _def.typeName Triage commit fd68ac6 added a `ProductSchema._def.typeName === 'ZodObject'` assertion based on the reviewer's follow-up suggestion. The reviewer's suggested probe was written against Zod 3 — Zod 4 dropped `_def.typeName` in favor of `_def.type` ('object' lowercase) and the codebase imports Zod 4. The assertion was failing because `_def.typeName` is `undefined`. Use `constructor.name === 'ZodObject'` instead — stable across both Zod major versions and the same closed-fail behavior the reviewer asked for. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * chore: regen schemas after rebase onto main with #1975 * chore: regen schemas after rebase onto main with #1981 --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Summary
Validation
Follow-ups