fix: preserve object helpers for schema intersections

[bokelley]

Summary

• Fix validate_property_delivery disappearing from TOOL_INPUT_SHAPES by regenerating object/object schema intersections as ZodObject .merge() where safe.
• Preserve .shape, .extend(), .omit(), and .pick() for generated schemas built from safe object intersections.
• Keep conflicting overlaps and richer combinator schemas as .and() so validation is not silently weakened.
• Add runtime, type-level, and generator-level regression coverage plus a patch changeset.

Validation

• npm run build:lib
• npm run typecheck
• NODE_ENV=test node --test-timeout=60000 --test-force-exit --test test/generate-zod-object-intersections.test.js test/lib/zod-schemas.test.js
• NODE_ENV=test node --test-timeout=60000 --test-force-exit --test test/lib/cli-webhook-receiver-flag.test.js
• NODE_ENV=test node --test-timeout=60000 --test-force-exit --test test/lib/conformance-cli.test.js
• npm test was attempted and ran 10,322 tests; it exited nonzero because test/lib/cli-webhook-receiver-flag.test.js and test/lib/conformance-cli.test.js hit the 60s per-file timeout under the combined run. Both passed when rerun individually.

Review

• JavaScript/DX review: no blocking concerns; requested negative generator coverage, now added.
• Code review: no blockers; requested the type-test be included, now added.

[aao-ipr-bot]

LGTM. Right layer for the fix — .and() → .merge() at codegen restores .shape/.extend/.omit/.pick for every downstream consumer, not just the MCP tool registration path that surfaced the bug.

Things I checked

• canSafelyMerge semantics (scripts/generate-zod-from-ts.ts:721). One-sided check is correct: allows disjoint keys, identical values, or left = right.optional() / right.nullish() — the canonical envelope-optional/child-required AdCP wrap pattern, where T.optional() ∩ T = T and right-wins merge produce the same shape. The inverse (left required, right optional) is correctly rejected.
• Fixpoint loop termination (scripts/generate-zod-from-ts.ts:825). Each non-fixed-point iteration strictly decreases .and( count → terminates.
• 48 substitutions in schemas.generated.ts all land on z.object(...).passthrough() or named *Schema resolvable to a z.object body on both sides. The 7 <union>.and(z.object(...)) sites (e.g. GetRightsResponseSchema, VerifyBrandClaimsResponseBulkSchema) correctly stay .and() because the union LHS yields undefined from schemaShapeForExpression. SearchBrandsResponseSchema correctly merges the two envelope wraps and leaves the trailing .and(z.union(...)) intact.
• isPlainZodObjectTail (scripts/generate-zod-from-ts.ts:622) only allows .passthrough() / .strict() / .strip() — verified the does not treat trailing-combinator schemas as ZodObject bases test in test/generate-zod-object-intersections.test.js covers the .refine() and trailing .and(z.union(...)) cases that would otherwise mis-merge.
• Load-bearing assertion (test/lib/zod-schemas.test.js:634-645). The Object.keys(TOOL_REQUEST_SCHEMAS).filter(t => !TOOL_INPUT_SHAPES[t]) parity check is the regression guard that would have caught validate_property_delivery disappearing the first time. Specifically asserts TOOL_INPUT_SHAPES.validate_property_delivery.list_id is non-undefined — exactly the field the MCP tool registration needed.
• Changeset severity — patch is correct. Bug fix restoring documented helper access, semantically equivalent rewrites, no public API addition.

Follow-ups (non-blocking — file as issues)

• Silent skip in TOOL_INPUT_SHAPES construction. src/lib/schemas/index.ts:72-75 does flatMap → [] when shapeOf returns undefined. The new test catches the TOOL_REQUEST_SCHEMAS codepath, but any other consumer of .shape on a non-mergeable intersection still silently degrades. Replace the silent drop with an explicit throw at TOOL_INPUT_SHAPES construction so future regressions surface loudly. (javascript-protocol-expert's recommendation.)
• Subsume earlier extractObjectSchema helper? The extractObjectSchema / ProductObjectSchema workaround from the recent ProductSchema DX fix solved the same problem class one layer up. Worth checking whether this codegen-level fix lets you deprecate it.
• isPlainZodObjectTail keep-in-sync. Currently allows only .passthrough() / .strict() / .strip(). If ts-to-zod ever starts emitting .catchall(z.unknown()) for an index signature, safe merges silently stop happening. One-line comment near scripts/generate-zod-from-ts.ts:622 pointing at ts-to-zod's tail emission would buy a heads-up later.

Minor nits (non-blocking)

1. Typed-export regex. scripts/generate-zod-from-ts.ts:738 uses (?::[^=]+)? for optional type annotations. Today every typed export is plain; if a future schema picks up a type annotation with = inside (default-typed generic), the export will silently fall out of the rewriter's view. Latent papercut, not a bug.
2. Test harness spawns npx tsx per test with mkdtempSync inside the repo root (test/generate-zod-object-intersections.test.js:11). Cold tsx startup × 3 noticeably extends npm test, and a SIGKILL mid-run leaves orphan .zod-object-intersections-* dirs in the working tree. os.tmpdir() would be safer, or import the rewriter as a real export and skip the spawn.

Safe to merge.

[bokelley]

Issue #1979 consolidates five non-blocking hardening items from review of this PR that overlap its diff. Lightweight enough to fold before merge if convenient — otherwise they'll resurface on #1979 post-merge.

1. Silent skip in TOOL_INPUT_SHAPES (src/lib/schemas/index.ts:72-75) — replace flatMap → [] with an explicit throw so future non-mergeable intersections surface loudly rather than degrading silently. Highest-signal item.
2. Subsume extractObjectSchema? — worth checking if this codegen-level fix lets the extractObjectSchema helper from feat(types): export ProductObjectSchema and extractObjectSchema to restore .extend() on intersection schemas #1971 be deprecated once both PRs land.
3. isPlainZodObjectTail keep-in-sync (scripts/generate-zod-from-ts.ts:622) — one-line comment near the tail-list pointing at ts-to-zod's .catchall(z.unknown()) emission as a future watch item.
4. Typed-export regex latent gap (scripts/generate-zod-from-ts.ts:738) — (?::[^=]+)? silently misses type annotations with = in a generic default; low priority but worth noting.
5. Test harness spawns npx tsx per test (test/generate-zod-object-intersections.test.js:11) — mkdtempSync in repo root leaves orphan dirs on SIGKILL; os.tmpdir() is safer.

Posted by the triage bot from #1979.

---

Generated by Claude Code

[aao-ipr-bot]

LGTM. Right fix at the right layer — the rewrite happens in codegen, so ZodObject helpers are restored at the source instead of papered over at every call site that needs .shape.

Things I checked

• Failure mode is real. src/lib/server/create-adcp-server.ts reads schema?.shape and continues on undefined, with only a logger.warn. ZodIntersection has no .shape, so TasksGetRequestSchema and ValidatePropertyDeliveryRequestSchema were silently skipped. New runtime test in test/lib/zod-schemas.test.js walks TOOL_REQUEST_SCHEMAS and asserts every entry has a matching TOOL_INPUT_SHAPES registration — that's the right gate.
• canSafelyMerge doesn't weaken validation in the accepted cases. The asymmetric optional check (left-optional + right-required is accepted; left-required + right-optional is rejected by string-compare) is correct: .and() of optional+required is required, .merge() of optional+required is required. The reverse (which would silently weaken) falls back to .and().
• z.infer<> stays stable. ZodIntersection<ZodObject<A>, ZodObject<B>> → A & B. ZodObject<A & B> → A & B. Adopters with explicit z.infer<typeof TasksGetRequestSchema> types see no observable change; they gain .shape navigability.
• Conservative tail allowlist (isPlainZodObjectTail) is sound. Only .passthrough() / .strict() / .strip() count as ZodObject-preserving. .refine(), .and(z.union(...)), .partial(), .catchall() correctly fall through to .and(). The negative generator test covers refined and union bases — good.
• Changeset present, patch is the right shape. Restores documented behavior (MCP tool registration), no API removal, no required-param flip. Runtime validation semantics unchanged at every site the safety check accepted.

Follow-ups (non-blocking — file as issues)

• unknownKeys policy isn't part of the safety gate. canSafelyMerge checks shape keys, not the tail policy. Today every rewrite is .passthrough().and(.passthrough()) so B-wins-on-merge is a no-op, but if ts-to-zod ever emits A.strict().and(B.passthrough()), the rewrite silently relaxes strictness. Either capture the tail in extractObjectLiteralBody and require equality, or just refuse the rewrite unless both sides end in .passthrough() (the only form ts-to-zod emits today). One-line guard, big future-proofing payoff.
• Convergence loop re-scans the whole file each pass. postProcessObjectIntersections:824-829 calls extractSchemaExports on every iteration of rewriteNamedObjectAnds. ~48 swaps × full file scan is fine at codegen scale today, sub-second, but the structure is quadratic. Worth memoizing the export map once and patching it after each rewrite if the generated file keeps growing.
• Test gaps worth backfilling. The optional/nullish-narrowing acceptance path (left z.string().optional() + right z.string()), the reverse-optional rejection, and a transitive C = A.and(B); D = C.and(...) case that proves the convergence loop actually converges. Three small cases pin down the most load-bearing branches of canSafelyMerge.

Minor nits (non-blocking)

1. readPropertyKey allows hyphens in bare identifiers at scripts/generate-zod-from-ts.ts:658. JS bare identifiers can't contain -. Today ts-to-zod doesn't emit invalid bare keys so it's never reached, but the regex invites a future footgun. Drop the - from the character class.
2. The optional-asymmetry in canSafelyMerge deserves a comment at scripts/generate-zod-from-ts.ts:721-734. The check looks asymmetric on the page; a two-liner explaining "merge means B wins, so right-optional would silently drop left's required constraint" would stop a future maintainer from "symmetrizing" it and re-introducing the bug.
3. Hand-rolled parser doesn't skip // or /* */. ts-to-zod doesn't emit comments inside schema expressions today, so this is theoretical, but a generator change could break it silently. A one-line assertion that the input contains no comment markers inside export const ...Schema = ... ; ranges would catch drift loudly.

Approving on the strength of the codegen-layer fix plus the type-test + runtime registration test, which together prevent this exact regression from reappearing.

[aao-ipr-bot]

LGTM. Real bug, sound fix, fail-closed gating. Restores .shape/.extend/.omit/.pick on ~48 generated schemas so validate_property_delivery and the other envelope/asset tools stop silently dropping out of TOOL_INPUT_SHAPES.

Things I checked

• canSafelyMerge (scripts/generate-zod-from-ts.ts:721-733) — the three accepted cases (identical, ${right}.optional(), ${right}.nullish()) all preserve validation. .and() requires both sides; .merge() keeps right. Intersection ∩ optional collapses to required, so the optional/nullish width-only allowances tighten correctly. Confirmed against TasksGetResponseSchema's triple-merge with ProtocolEnvelopeSchema.task_id.optional() vs the right's required task_id — both forms resolve to required.
• isPlainZodObjectTail (scripts/generate-zod-from-ts.ts:622-633) — rejects bases ending in .refine() or .and(z.union(...)), only allows .passthrough()/.strict()/.strip(). Fail-closed. Negative test at test/generate-zod-object-intersections.test.js:79-104 locks it.
• Integration order at scripts/generate-zod-from-ts.ts:1046 — runs after postProcessForPassthrough (so .passthrough() tails are present for the predicate) and before the second postProcessUndefinedUnions (no structural conflict). Also before postProcessTS7056Annotations, which matters because the export regex (?::[^=]+)? would otherwise have to deal with type annotations.
• Fixpoint loop in postProcessObjectIntersections — rewriteNamedObjectAnds is monotonically non-increasing in \w+Schema.and( count; cannot diverge.
• Unknown-key semantics — both envelope schemas (AdCPVersionEnvelopeSchema, ProtocolEnvelopeSchema) are z.object({...}).passthrough(), and the rewritten literal re-wraps .passthrough() on the merge argument. Passthrough is preserved.
• JSON Schema output via toJSONSchema() — .merge() produces flat {"type":"object","properties":...} instead of allOf, which is exactly the shape MCP clients prefer. Existing blanket regression test at test/lib/zod-schemas.test.js:557-582 would have caught conversion failures across the ~48 flipped schemas.
• Type test (src/type-tests/zod-object-intersections.type-test.ts) — exercises .shape/.extend/.omit/.pick on ValidatePropertyDeliveryRequestSchema, TasksGetRequestSchema, TasksGetResponseSchema, IndividualImageAssetSchema, GroupVideoAssetSchema, CreativeVariantSchema.
• Runtime test (test/lib/zod-schemas.test.js:606-646) — full TOOL_REQUEST_SCHEMAS → TOOL_INPUT_SHAPES coverage check with explicit validate_property_delivery.list_id assertion. Locks the bug.
• Changeset is a patch. Right call — runtime validation for valid inputs is unchanged; the helpers are documented behavior being restored, not new API surface.
• src/lib/version.ts 8.1.0-beta.6 → 8.1.0-beta.8 is a routine LIBRARY_VERSION sync (matches the prior chore: sync LIBRARY_VERSION merge); package.json is not touched.

Follow-ups (non-blocking — file as issues)

• scripts/generate-zod-from-ts.ts:769-798 — schemaShapeForExpression doesn't recognize .merge(...) chains. After the first pass converts A = B.and(z.object({...})) to A = B.merge(z.object({...})), any later C = A.and(z.object({...})) cannot resolve A's shape and stays as .and(). Conservative (no correctness loss), but if a future tool drops out of TOOL_INPUT_SHAPES due to a chained envelope schema, this is the seam. Cheap fix: also recognize <expr>.merge(z.object({...})) and union the parsed shapes.
• test/generate-zod-object-intersections.test.js — no explicit unit case for the width-only-overlap rule (left = ${right}.optional()). Exercised indirectly via the TasksGet runtime test; a direct unit test would lock canSafelyMerge against upstream schema drift.
• scripts/generate-zod-from-ts.ts:1046 — worth a one-line comment noting the position is load-bearing ("must run after postProcessForPassthrough and before postProcessTS7056Annotations") — symmetric to the existing ordering comments above.

Minor nits (non-blocking)

1. Scanner domain. scripts/generate-zod-from-ts.ts:526-566 — the regex-literal heuristic is correct because the input is always post-ts-to-zod Zod source (no division operators in expression position), but a one-line comment naming that domain would deter future editors from repurposing the scanner against arbitrary TypeScript.

The unrelated npm test timeout on cli-webhook-receiver-flag.test.js and conformance-cli.test.js is noise — both pass when rerun individually and the change here doesn't touch either path.

Approving on the strength of the fail-closed canSafelyMerge gate plus the dual type-level + runtime regression coverage.

[bokelley]

Folded items 1, 3, and 5 from #1979 into this branch (commit 976e67d):

• Item 1 — TOOL_INPUT_SHAPES silent skip → throw (src/lib/schemas/index.ts): replaced flatMap(… return []) with .map(… throw Error(…)) so a schema without .shape fails loudly at module init rather than silently disappearing from the map.
• Item 3 — isPlainZodObjectTail watchdog comment (scripts/generate-zod-from-ts.ts:622): added a one-liner noting that ts-to-zod's .catchall(z.unknown()) emission should be added to objectPreservingMethods if it ever appears.
• Item 5 — mkdtempSync prefix (test/generate-zod-object-intersections.test.js): switched from REPO_ROOT to os.tmpdir() so the harness directory lands outside the working tree and doesn't orphan on SIGKILL.

Deferred to #1979 post-merge:

• Item 2 — whether extractObjectSchema from feat(types): export ProductObjectSchema and extractObjectSchema to restore .extend() on intersection schemas #1971 can now be deprecated (design question, worth a focused look once both PRs are in).
• Item 4 — (?::[^=]+)? regex latent gap for generic defaults with = (low-priority, tracked in Codegen: hardening follow-ups for Zod ergonomics passes (post #1972/#1975) #1979).

All three object-intersection tests pass locally with the changes in place.

---

Generated by Claude Code

[bokelley]

Acknowledged — commit 976e67d is on the branch head with items 1, 3, and 5 folded in. Items 2 and 4 tracked in #1979 for post-merge follow-up.

---

Triaged by Claude Code.

---

Generated by Claude Code

[aao-ipr-bot]

Generator-only fix to expose .shape/.extend/.omit/.pick on schemas that ts-to-zod was emitting as ZodIntersection. Right diagnosis: the silent flatMap-skip in TOOL_INPUT_SHAPES was hiding validate_property_delivery (and the 13 other envelope-inheriting tools). Right shape: the conservative canSafelyMerge keeps conflicting overlaps and trailing-union/refine bases as .and(), and the new module-init throw means the next regression gets caught at startup, not in production.

Hold: one-character typo at scripts/generate-zod-from-ts.ts:33

return content.replace(/(?<!\.\.never\(\))\.optional\(\)/g, '.nullish()');

The extra \. makes the lookbehind require ..never() (two dots) before .optional(), which is unreachable. The original (?<!\.never\(\)) did real work — the comment immediately above says z.never().optional() must stay as-is because converting it to .nullish() weakens "must not be provided" to "null is fine." Currently latent because nothing in schemas.generated.ts matches z.never() today, but the next schema that introduces it will silently get rewritten. One-character fix; please fold into this PR rather than file a follow-up.

Things I checked

• postProcessObjectIntersections parser: skipQuotedOrRegexLiteral handles strings and regex literals, balanced scan covers ()/{}/[]. Template literals with ${} are not explicitly handled, but ts-to-zod doesn't emit them inside schema bodies.
• isPlainZodObjectTail whitelist (.passthrough/.strict/.strip) leaves .refine/.transform/trailing .and(union) as ZodIntersection — test case 3 in test/generate-zod-object-intersections.test.js pins this.
• canSafelyMerge asymmetry is sound: left=X.optional()+right=X is allowed (right wins, tightens to X); reverse is rejected (would loosen). Conservative-failing is the right posture — a missed-safe-merge becomes a module-init throw caught by CI, not a silent regression.
• rewriteNamedObjectAnds fixed-point loop terminates monotonically; cycle guard via visiting in schemaShapeForExpression correctly threaded.
• SearchBrandsResponseSchema at schemas.generated.ts:9220 — first .and(ProtocolEnvelopeSchema) merged, second .and(z.union([Success, Error])) correctly kept as .and().
• Fail-closed at src/lib/schemas/index.ts:74-77. Module-init throw beats the previous silent flatMap-skip. Error message names both the key and the remedy ("use merge() not and()").
• All 48 .and(→.merge( rewrites in schemas.generated.ts. None of the affected envelope-inheriting schemas (Tasks*, ValidatePropertyDelivery*, ListCreativeFormatsRequestCreativeAgent, SearchBrandsResponse, ListAccountsResponse, 11 Group*Asset / Individual*Asset) have spec-defined allOf semantics — these are pure envelope/base inheritance and flat-merge is semantically equivalent.
• 236-line inline-enums.generated.ts refresh matches ADCP_VERSION = '3.1.0-beta.3' — expected beta.3 cache drift (CanonicalFormat* family, query_upstream_traffic scenario, CreativePurgedWebhook.initiator, the asset_source/audio_channels deprecated-alias regeneration), not unexpected churn.
• Wire equivalence: framework tools register with PASSTHROUGH_INPUT_SCHEMA, so this only affects customToolFor adopters who consume TOOL_INPUT_SHAPES directly. For them, previously-skipped tools become available with proper shape rather than missing. Recovery, not regression.
• Changeset present, patch-level appropriate, prose names the user-visible effect.

Follow-ups (non-blocking — file as issues)

• Cross-import regex tightening at scripts/generate-zod-from-ts.ts:969 (;?\n+ → ; ?\n+) is unrelated to the stated fix. The defensive throw at lines 975-981 catches any silent no-op, so the practical risk is bounded — but the change still loosens robustness against an injector that ever omits the semicolon. Restore ;? or document the requirement.
• Header literal at line ~1078 was flattened from multi-line template to single-line \n escapes. Drive-by cosmetic with no behavioral effect; pick one form and stop churning it.
• isPlainZodObjectTail watch comment correctly flags .catchall(z.unknown()) as a future ts-to-zod output. The validate_property_delivery should be registered assertion at test/lib/zod-schemas.test.js:637 is a good spot-check; a cardinality canary (Object.keys(TOOL_REQUEST_SCHEMAS).length === Object.keys(TOOL_INPUT_SHAPES).length) would catch the same class of regression without needing to enumerate keys.

Ship after the line-33 typo is folded in.

[aao-ipr-bot]

LGTM. Right call to fold the silent-skip into a throw — fail-closed beats fail-open, and the missing validate_property_delivery shape proves the prior flatMap was hiding a real bug.

Things I checked

• canSafelyMerge at scripts/generate-zod-from-ts.ts:722-735 is symmetric in the right direction: narrowing (left optional, right required) merges; widening (left required, right optional) stays as .and(). Both cases preserve AND-semantics under .passthrough().
• .and(z.union(...)) patterns in schemas.generated.ts are correctly left alone — schemaShapeForExpression returns undefined for unions, so rewriteTopLevelObjectAnds falls through to .and(). Verified SearchBrandsResponseSchema = AdCPVersionEnvelopeSchema.merge(ProtocolEnvelopeSchema).and(z.union(...)) at schemas.generated.ts:9220 — the trailing union is preserved.
• isPlainZodObjectTail whitelists exactly .passthrough() / .strict() / .strip() — anything richer (refine, transform, and-with-union) blocks the merge. The watchdog comment about .catchall(z.unknown()) at scripts/generate-zod-from-ts.ts:622 is the right discipline.
• schemaShapeForExpression recursion is bounded by the visiting set at scripts/generate-zod-from-ts.ts:786, so circular named refs return undefined instead of looping.
• Test harness at test/generate-zod-object-intersections.test.js:11-39 uses mkdtempSync in os.tmpdir() with force: true, recursive: true cleanup — isolated and idempotent.
• All 51 entries in TOOL_REQUEST_SCHEMAS resolve to ZodObjects with .shape today; the throw at src/lib/schemas/index.ts:73-77 is enforced by the runtime test at test/lib/zod-schemas.test.js:633-644. The invariant has a test, not just hope.
• Changeset is patch — correct. The schemas are equivalent or narrower under merge, no public API removal, and the throw exposes a previously-silent bug rather than introducing a regression.

Follow-ups (non-blocking — file as issues)

• The while (true) fixpoint loop at scripts/generate-zod-from-ts.ts:826-830 is effectively a single pass — once a schema is rewritten to Foo.merge(z.object(...)) its expression no longer matches the bare-named-ref or inline-z.object patterns that schemaShapeForExpression resolves, so subsequent passes never gain new shape info. Termination is guaranteed (strict reduction of .and( count) but the loop is misleading. Either drop the loop or add a one-liner explaining the intended iteration semantics.
• Extend isPlainZodObjectTail to accept .catchall(z.unknown()) now rather than waiting for ts-to-zod to emit it. The watchdog comment is good; the proactive whitelist is better.
• Changeset body could add "may surface previously-silent misconfigurations" so adopters with custom schema augmentations see the throw coming.

Minor nits (non-blocking)

1. Export-regex assumption. /export const (\w+Schema)(?::[^=]+)? = /g at scripts/generate-zod-from-ts.ts:739 skips type annotations using [^=]+. Works today because no TS7056 annotation contains =. A future defaulted generic (<T = unknown>) would mis-match. Worth a short comment naming the assumption.
2. Module-load throw audibility. src/lib/schemas/index.ts:73-77 kills the whole import * as schemas from '@adcp/sdk/schemas' if any entry lacks .shape. That's correct fail-closed behavior, but a one-line comment near the throw stating "invariant enforced by test/lib/zod-schemas.test.js" would save the next reader a grep.

Approving on the strength of the merge-vs-AND analysis plus the fail-closed throw. The fixpoint-loop observation is interesting but not load-bearing.