Skip to content

fix(server): tighten handler return types so schema drift fails tsc#764

Merged
bokelley merged 2 commits into
mainfrom
bokelley/tighten-union-types
Apr 22, 2026
Merged

fix(server): tighten handler return types so schema drift fails tsc#764
bokelley merged 2 commits into
mainfrom
bokelley/tighten-union-types

Conversation

@bokelley

Copy link
Copy Markdown
Contributor

Summary

Closes #727 (B). Makes sparse handler returns like { rights_id, status: 'acquired' } fail tsc at compile time against AcquireRightsResponse, instead of only failing at wire-level validation.

Two hand-written type bindings in create-adcp-server.ts were routing around the (already-tight) generated discriminated unions:

  • AdcpToolMap brand-rights slots were typed result: Record<string, unknown> — stale scaffold from before the response types were code-generated. Swapped for the generated success types (AcquireRightsAcquired | AcquireRightsPendingApproval | AcquireRightsRejected, GetRightsSuccess, GetBrandIdentitySuccess).
  • DomainHandler's return union included | Record<string, unknown> as a general escape hatch for every tool. Removed. Handler returns are now AdcpToolMap[K]['result'] | McpToolResponseadcpError(...) still works unchanged.

No upstream schema or generator changes needed. The looseness was purely in SDK plumbing; the generated types were already correct discriminated unions with proper required arrays.

What this surfaced

Removing the escape hatch caught two real latent bugs in the reference test agents:

  • test-agents/seller-agent.ts: PRODUCTS were missing reporting_capabilities (required on Product). Strict response validation (default in dev/test per feat(server): strict response validation default in dev/test (#727) #757) would have caught this at runtime; now it fails at build. Fixed with DEFAULT_REPORTING_CAPABILITIES.
  • test-agents/seller-agent-signed-mcp.ts: createMediaBuy was reading pkg.package_id from the request, but PackageRequest has no such field per spec — buyers send buyer_ref and the seller mints package_id. Fixed to mint crypto.randomUUID().

Regression guard

test/handler-return-tightness.test.js asserts (a) no brand-rights slot in AdcpToolMap is typed Record<string, unknown> and (b) DomainHandler's return union does not contain Record<string, unknown>. The runtime suite alone can't catch a reintroduced compile-time escape hatch.

Test plan

  • npm run build:lib — clean
  • npm run build:test-agents — clean (was 2 errors before the test-agent fixes)
  • npm run test:lib — 4391 pass, 0 fail
  • Verified with @ts-expect-error harness: sparse shapes, empty objects, and { anything: 'goes' } all fail tsc at BrandRightsHandlers['acquireRights'] return position; valid shapes compile
  • code-reviewer + security-reviewer feedback addressed

Migration

Minor bump. If a handler returns a plain object literal without the full success shape, tsc will now flag it. Either fill in the missing required fields (use DEFAULT_REPORTING_CAPABILITIES for Product.reporting_capabilities) or wrap with a response builder (productsResponse({ ... }), acquireRightsResponse({ ... })) — details in the changeset.

🤖 Generated with Claude Code

bokelley and others added 2 commits April 21, 2026 23:04
`AdcpToolMap` brand-rights result slots were typed `Record<string,
unknown>` (stale scaffold) and `DomainHandler`'s return union
carried a `| Record<string, unknown>` escape hatch. Together these
let sparse handler returns like `{ rights_id, status: 'acquired' }`
pass tsc against `AcquireRightsResponse` — drift only failed at
wire-level validation.

Swaps brand-rights slots to the generated success types, drops the
escape hatch from `DomainHandler`, and adds a regression test that
asserts neither loosening can return. Test agents updated alongside:
`seller-agent.ts` uses `DEFAULT_REPORTING_CAPABILITIES`, and the
signed-MCP agent now mints `crypto.randomUUID()` for `package_id`
instead of reading it from an untyped request field (latent bug —
buyers don't send `package_id`).

Closes #727 (B).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@bokelley bokelley merged commit f64007c into main Apr 22, 2026
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Tighten generated types + default response validation to catch schema drift at the right layer

1 participant