Skip to content

feat(server): idempotency: 'disabled' mode + standalone enum value arrays#931

Merged
bokelley merged 4 commits into
mainfrom
bokelley/sdk-test-ergonomics
Apr 25, 2026
Merged

feat(server): idempotency: 'disabled' mode + standalone enum value arrays#931
bokelley merged 4 commits into
mainfrom
bokelley/sdk-test-ergonomics

Conversation

@bokelley

Copy link
Copy Markdown
Contributor

Summary

Two additive surfaces for the @adcp/client SDK aimed at consumers who currently UUID-inject every test payload to satisfy AdCP 3.0's idempotency_key requirement, or who duplicate spec enum literals in their own validation code.

1. idempotency: 'disabled' for createAdcpServer

AdcpServerConfig.idempotency now accepts 'disabled' | IdempotencyStore. When set to 'disabled':

  • Capability declaration flips to the spec's IdempotencyUnsupported branchget_adcp_capabilities returns adcp.idempotency: { supported: false } (no replay_ttl_seconds), matching the oneOf discriminator in the spec. Buyers reading caps fall back to natural-key dedup before retrying spend-committing operations.
  • The replay middleware (INVALID_REQUEST / IDEMPOTENCY_CONFLICT / IDEMPOTENCY_EXPIRED) is skipped.
  • Schema validation tolerates a missing idempotency_key on mutating tools — surgically, only the keyword: 'required', pointer: '/idempotency_key' issue is dropped; every other required field still produces VALIDATION_ERROR.
  • A pre-middleware shape gate enforces IDEMPOTENCY_KEY_PATTERN whenever a key IS supplied, regardless of validation mode — defense-in-depth so malformed keys never reach handler logs.
  • Throws at construction under NODE_ENV=production (silent double-execution on retry is a money-flow incident waiting to happen). Outside production, a logger.warn fires.

Production servers must still wire a real store via createIdempotencyStore({ backend, ttlSeconds }). The existing idempotency: store and idempotency: undefined paths are unchanged.

2. Standalone enum value arrays

New generated file src/lib/types/enums.generated.ts exports a ${TypeName}Values const array for every named string-literal union in the AdCP TypeScript types (122 enums: MediaChannelValues, PacingValues, MediaBuyStatusValues, DeliveryTypeValues, AssetContentTypeValues, …).

import { MediaChannelValues } from '@adcp/client/types';
const channels = new Set<string>(MediaChannelValues);
if (!channels.has(input)) throw new Error('unknown channel');

Codegen wired into npm run generate-types so ci:schema-check catches drift the same way it catches type drift. Inline anonymous unions (e.g., formats?: ('jpg' | ...)[]) are out of scope — they don't have stable names in the generated TypeScript.

Expert review caught a wire-level blocker

Initial implementation kept supported: true in get_adcp_capabilities even in disabled mode — three expert reviews (ad-tech-protocol-expert, code-reviewer, security-reviewer) converged on this as a money-flow footgun: a buyer reading caps and retrying after a 504 would double-book real spend. The spec's IdempotencyUnsupported branch (supported: false, no replay_ttl_seconds) is the correct shape and is now what disabled mode emits.

Other expert findings addressed: throw-on-production, pre-middleware shape gate, pinned the pointer-match invariant in a comment, bumped enum codegen floor from <20 to <100 to catch partial regex drift.

Test plan

  • Unit tests in test/server-idempotency.test.js (+6): warn at construction, throw under NODE_ENV=production, schema filter is surgical (other required fields still fail), no replay, capability flips to supported: false, shape gate rejects malformed keys
  • Wire-level e2e in test/server-idempotency-disabled-e2e.test.js (+5): real serve() + official @modelcontextprotocol/sdk Client over StreamableHTTPClientTransport; A2A roundtrip via createA2AAdapter; both validation: 'strict' and validation: 'off' paths
  • Cross-validation tests in test/lib/enum-arrays.test.js (+6): every Values array round-trips against its matching Zod Schema; Pacing and MediaBuyStatus literals match the spec; ≥30 enum/schema pairs cross-checked
  • npm run test:lib: 4941 pass, 0 fail, 6 skipped (pre-existing)
  • Typecheck clean (tsc --noEmit --project tsconfig.lib.json)
  • Reviewer: confirm storyboard runner auto-skip on supported: false is a follow-up (not a blocker) — compliance/.../universal/idempotency.yaml says supported: false sellers MUST skip, but the runner doesn't auto-detect today

Follow-ups (not in this PR)

  • Storyboard runner auto-skip when agent declares idempotency.supported: false (file an issue)
  • Inline anonymous enum extraction (image asset formats, video container formats etc. — different extractor needed since they don't have stable names)

🤖 Generated with Claude Code

…rays

Two additive surfaces aimed at consumers who currently UUID-inject every
test payload to satisfy AdCP 3.0's idempotency_key requirement, or who
duplicate spec enum literals in their own validation code.

1. `idempotency: 'disabled'` for createAdcpServer. Flips the agent into
   the spec's `IdempotencyUnsupported` capability branch ({supported: false},
   no replay_ttl_seconds) so wire advertisement matches actual behavior.
   Skips the replay middleware, surgically tolerates missing idempotency_key
   in schema validation (only the `required + /idempotency_key` issue is
   filtered — every other required field still fails VALIDATION_ERROR),
   and runs a pre-middleware shape gate so malformed keys are still
   rejected with INVALID_REQUEST regardless of validation mode. Throws at
   construction under NODE_ENV=production to prevent the money-flow
   footgun where disabled mode silently double-executes mutating handlers
   on retry.

2. Standalone enum value arrays. New generated file
   src/lib/types/enums.generated.ts exports a `${TypeName}Values` const
   array for every named string-literal union in the spec (122 enums:
   MediaChannelValues, PacingValues, MediaBuyStatusValues,
   AssetContentTypeValues, etc.). Adapters can import authoritative
   literal sets without re-deriving from Zod schemas. Codegen wired into
   `npm run generate-types` so ci:schema-check catches drift the same
   way it catches type drift.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Comment thread scripts/generate-enum-arrays.ts Fixed
bokelley and others added 3 commits April 24, 2026 21:13
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
CodeQL `js/incomplete-sanitization` flagged the hand-rolled
single-quote-only escape — backslashes weren't escaped, so a value
like `it\'s` would render as two tokens. Spec enums are all simple
alphanumerics today so the bug never fires, but JSON.stringify is
the boring correct choice and future-proofs against any spec value
that ever contains punctuation.

Output now uses double-quoted literals, which prettier ignores
(`*.generated.ts` is prettierignored).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Previous gate refused only under NODE_ENV === 'production', but NODE_ENV
is unset by default in raw Lambda, custom containers, and many K8s
deployments — exactly the environments where a silent disabled-mode
start is most dangerous. Inverted to an allowlist: throw unless
NODE_ENV is explicitly 'test' or 'development', or the operator sets
ADCP_IDEMPOTENCY_DISABLED_ACK=1 to acknowledge the risk. Tests cover
unset NODE_ENV, 'staging' (custom env names), the ack escape hatch,
and strict ack-value matching (only literal '1' acknowledges, not
'true'/'yes').

Also drop the // Generated at: timestamp from enums.generated.ts so
regenerations don't produce noisy diffs — git log + .generated.ts
suffix are sufficient signal that the file is tool-output. Other
generated files in the repo keep their existing timestamp convention;
this is a per-file choice for new codegen.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@bokelley

Copy link
Copy Markdown
Contributor Author

Addressed P0 + P2 hygiene; filed P1 as #932.

P0 — production gate inverted (76db06f). Allowlist NODE_ENV{'test', 'development'}; anything else (unset, 'production', 'staging', custom names) throws unless ADCP_IDEMPOTENCY_DISABLED_ACK=1. Strict ack-value matching — only literal '1' acknowledges, not 'true'/'yes' (prevents copy-paste typos).

New tests cover: NODE_ENV unset → throws (the actual Lambda case), 'staging' → throws, ack escape works, ack only accepts '1'.

P2.1 — generated-at timestamp dropped from enums.generated.ts (76db06f). New file, no in-band timestamp; git log + .generated.ts suffix are sufficient signal. Other generated files keep their existing timestamp convention (out-of-scope to refactor).

P2.2 — AgeVerificationMethod1Values: investigated. Both AgeVerificationMethod and AgeVerificationMethod1 are real types in core.generated.ts — json-schema-to-typescript emits the suffix when the same enum is referenced twice from different schema paths. Both are validly importable, and dedup-by-value would arbitrarily drop one and break the corresponding consumer import. Keeping as-is; the cosmetic fix is upstream in json-schema-to-typescript.

P2.3 — NODE_ENV mutation race: documented in a comment at the top of the production-gate test block. The before/after-with-finally pattern is safe under the current node:test serial-within-file default; if --test-concurrency ever raises that, we move to a child process.

P1 — inline anonymous unions filed as #932. The clean fix is Zod-introspection (named schemas expose .options / .def.options on union types) rather than regex-on-TS — different tool, different PR. Acceptance criteria + suggested naming convention in the issue.

@bokelley bokelley merged commit 942acda into main Apr 25, 2026
9 checks passed
bokelley added a commit that referenced this pull request Apr 25, 2026
…nions (#932) (#938)

* feat(types): inline-union value arrays for anonymous string-literal unions

Closes #932. Companion to the named-enum exports landed in 5.17 (#931).
Where the earlier shipment covered every spec enum with a stable named
type (`MediaChannelValues`, `PacingValues`, etc.), this adds the inline
anonymous unions inside named schemas — exactly the cases where adapters
were re-declaring spec literal sets (image formats, video containers,
audio formats, codecs, channels, etc.) as drift bait in their own code.

Naming: every `z.union([z.literal(...), ...])` inside a named object
schema gets `${ParentSchema}_${PropertyName}Values` (PascalCased).
Property paths that reference a named enum (e.g. `unit:
DimensionUnitSchema.optional()`) are intentionally skipped — those
already have `${TypeName}Values` exports from `enums.generated.ts`.

Implementation: `scripts/generate-inline-enum-arrays.ts` walks the
compiled Zod schemas via runtime introspection (Zod 4 `_def`) rather
than regex on the generated TS — cleaner and future-proofs against
codegen output format changes. Wired into `generate-zod-schemas` so it
runs after Zod codegen. New test cross-validates every emitted array
against the parent Zod schema property — drift in either side fails
fast.

104 inline-union arrays across 51 parent schemas, including all
user-flagged cases (Image/Video/AudioAssetRequirements_FormatsValues,
VideoAssetRequirements_ContainersValues / _CodecsValues, etc.).

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

* fix(codegen): address expert review findings on inline-enum extractor

Three review-driven hardenings:

1. Drop `'pipe'` from UNWRAP_TYPES. `z.pipe(in, out)` chains transforms
   and the inner `in` schema may not match the wire-accepted set after
   `out` runs — unwrapping it would silently extract the wrong half for
   `string → enum` style transforms. Current `schemas.generated.ts` has
   no `z.pipe(...)` constructions; if a future codegen change adds one,
   the extractor now correctly bails at a non-`union` core rather than
   emitting a wrong literal set, and the bumped floor catches the count
   collapse.

2. Add fingerprint-based fallback to the named-enum dedup gate.
   Identity-based check is exact and fast in the common case (Zod
   re-uses schema instances on property access), but breaks if codegen
   ever clones — e.g., a future `json-schema-to-zod` pass that inlines
   a referenced type as a fresh union. The fingerprint set keyed by
   sorted-literal-tuple catches that regression and prevents duplicate
   emissions of named-enum values under wrapped property names. This
   correctly drops 4 entries that previously double-emitted named
   enums under inline names: ListAccountsRequest_StatusValues
   (== AccountStatusValues), ReportingWebhook_ReportingFrequencyValues
   (== ReportingFrequencyValues), VASTAssetRequirements_VastVersionValues
   (== VASTVersionValues), WebhookAssetRequirements_MethodsValues
   (== HTTPMethodValues). Net: 100 inline arrays, no consumer-facing
   regression (each dropped name's values are reachable via the
   matching named-enum export).

3. Bump floor from <20 to <90. Catches partial-regression scenarios
   the previous floor only caught for catastrophic collapse.

Plus test improvements:
- Cross-validation regex switched from first-underscore split to
  last-underscore split, so parents with SCREAMING_SNAKE prefixes
  (e.g. `RATE_LIMITEDDetails`) get cross-validated instead of
  silently skipped.
- New negative-case test pins the invariant that mixed-type unions
  (string + number, all-boolean) are skipped — defensive against a
  future spec change adding non-string members to a string-literal
  enum.

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

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@github-actions github-actions Bot mentioned this pull request Apr 25, 2026
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.

2 participants