Skip to content

feat(sdk): address 5.15.0 DX feedback on handler types, error shape, extraction#874

Merged
bokelley merged 2 commits into
mainfrom
bokelley/sdk-feedback-5-15
Apr 24, 2026
Merged

feat(sdk): address 5.15.0 DX feedback on handler types, error shape, extraction#874
bokelley merged 2 commits into
mainfrom
bokelley/sdk-feedback-5-15

Conversation

@bokelley

Copy link
Copy Markdown
Contributor

Summary

Addresses five user-reported DX issues on @adcp/client 5.15.0 (full feedback here). All changes additive on the wire; no breaking migrations required.

  • Root re-exports for Success/Error/Submitted arms of every *Response union (SyncCreativesSuccess, SyncAudiencesSuccess, CreateMediaBuySuccess, BuildCreativeSuccess, BuildCreativeMultiSuccess, ActivateSignalSuccess, SyncAccountsSuccess, SyncGovernanceSuccess, UpdateContentStandardsSuccess, ProvidePerformanceFeedbackSuccess, SyncEventSourcesSuccess, LogEventSuccess, etc., plus Error and Submitted variants) — AdcpServer handler returns no longer need as any.
  • Dual-export idempotency helpers (createIdempotencyStore, memoryBackend, pgBackend, plus types) from @adcp/client root, matching createAdcpServer.
  • Widened DomainHandler return type to accept Promise<Success | FullResponseUnion | McpToolResponse>. The dispatcher narrows Submitted (status: 'submitted' && task_id) and Error arms ({errors, context, ext, success, conflicting_standards_id}) at runtime so success-arm response builders only run on actual Success payloads. Adapter patterns returning Result<CreateMediaBuyResponse, ...> type-check without casts.
  • extractResult<T>(result) helper — prefers structuredContent, falls back to JSON-parsing content[0].text, returns undefined. Lightweight companion to unwrapProtocolResponse (which throws and schema-validates).
  • VALIDATION_ERROR.issues surfaced at top level of adcp_error so operators see JSON Pointers immediately. Same list mirrored at details.issues for spec-convention compatibility. Request-side schemaPath gating now threads exposeSchemaPath the same way response-side does.

Also: lightweight dev-log warning on handler-returned Error arms whose errors[] items violate the spec (missing code/message).

Changes

  • src/lib/index.ts — root re-exports (types + idempotency helpers)
  • src/lib/server/create-adcp-server.tsAdcpToolMap gains response field; DomainHandler widened; new isSubmittedEnvelope / isErrorArm / wrapSubmittedEnvelope / wrapErrorArm; aligned schemaPath gating
  • src/lib/server/errors.tsAdcpErrorOptions / AdcpErrorPayload gain top-level issues
  • src/lib/validation/schema-errors.ts — emits issues at both top-level and details.issues
  • src/lib/utils/extract-result.ts — new helper
  • docs/guides/BUILD-AN-AGENT.md — response-union narrowing, extractor asymmetry, Error-arm vs adcpError guidance
  • docs/migration-4.x-to-5.x.md — refresh the three details.issues callouts

Review

Addressed feedback from code-reviewer, security-reviewer, ad-tech-protocol-expert, dx-expert, and nodejs-testing-expert. Key decisions:

  • Error arm vs adcp_error envelope: kept separate. Spec-defined per-tool failures → return the Error arm directly. Framework/infra failures → adcpError('CODE', ...). Documented side-by-side in BUILD-AN-AGENT.
  • VALIDATION_ERROR wire shape: rather than moving issues (which would have broken buyers reading details.issues), added a top-level copy AND kept the details.issues mirror. Additive, not breaking.
  • Error-arm shape check: dev-log warning, not a framework-level flip to VALIDATION_ERROR. Preserves the seller's intent while surfacing drift.

Test plan

  • tsc --noEmit clean
  • npm run build clean
  • 606/606 targeted regression battery passes (server dispatcher, idempotency, responses, errors, envelope wrap, schema validation, response unwrapper, type safety, discriminated unions, uniform-error comparator)
  • 10 new dispatcher tests covering: Error/Submitted/Success arm narrowing, Submitted-with-advisory-errors (ordering), Error arm with context/ext siblings, empty errors[], unknown-key fallthrough, adcpError() on non-VALIDATION_ERROR code not emitting issues, and dual-location issues on the wire
  • CI green

🤖 Generated with Claude Code

bokelley and others added 2 commits April 24, 2026 08:47
…extraction

Five user-reported DX issues on @adcp/client 5.15.0:

- Root re-exports for every *Response union's Success / Error / Submitted arms
  (SyncCreativesSuccess, SyncAudiencesSuccess, etc.), so handlers no longer need
  `as any` when narrowing.
- Dual-export createIdempotencyStore / memoryBackend / pgBackend (and related
  types) from the root, matching createAdcpServer's treatment.
- Widened DomainHandler to accept Success | full Response union | McpToolResponse.
  Dispatcher narrows Submitted (status: 'submitted' + task_id) and Error arms
  ({errors, context, ext, success, conflicting_standards_id}) at runtime so
  the response builder only applies Success-shape defaults (revision,
  confirmed_at, ...) to actual Success payloads. Adapter patterns returning
  `Result<CreateMediaBuyResponse, ...>` type-check without casts.
- extractResult<T>(result) helper — prefers structuredContent, falls back to
  JSON-parsing content[0].text, returns undefined. Lightweight companion to
  unwrapProtocolResponse (which throws and schema-validates).
- VALIDATION_ERROR.issues now surfaced at the top level of adcp_error so
  operators see JSON Pointers on first render. Same list mirrored at
  details.issues for spec-convention compatibility — existing readers
  continue to work, no migration required. Request-side schemaPath gating
  now threads exposeSchemaPath the same way response-side does (previously
  stripped even in dev).

Also: lightweight spec-shape warning on handler-returned Error arms whose
errors[] items are missing `code` or `message`.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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.

1 participant