fix: server handler payload aliases

[bokelley]

Summary

• Add named server *Payload and *HandlerResult aliases across decisioning surfaces and re-export them from @adcp/sdk/server.
• Keep server payload annotations response-safe by stripping protocol envelope fields plus write-only webhook credentials and billing bank coordinates.
• Align raw response builders with the public payload types by stripping nested account credentials/bank fields and media-buy invoice-recipient bank fields at runtime.
• Expand repo and packed-adopter type guards for Result<Payload, E>, TaskHandoff, account handler aliases, broad alias exports, and write-only field stripping.

Root Cause

The previous tests covered direct SalesPlatform payload returns, but not adopter helper layers that annotate results as Result<GeneratedWireResponse, E>. Those generated response types include protocol task-envelope fields such as status: TaskStatus, so adopter helpers were forced to return envelope fields that the SDK owns and stamps later.

During expert review, we also found that widening public payload aliases needed to stay aligned with the write-only response contract for nested webhook credentials and bank details. This PR fixes both the type surface and the raw response-builder runtime projection.

Expert Review

• Protocol review flagged the raw response-builder runtime/type mismatch for nested account credentials and bank fields; fixed with runtime stripping and regression tests.
• Security review flagged write-only authentication.credentials and BusinessEntity.bank exposure risk in public aliases; fixed in ServerPayload<T> and guarded by type tests.
• Code review flagged no remaining blockers after the runtime/type fixes.
• Product review asked for broader alias coverage, account handler alias guards, and migration guidance; all added.

Validation

• npm run typecheck
• npm run build:lib
• node --test test/server-responses.test.js test/lib/account-notification-config-projection.test.js
• npm run check:adopter-types
• git diff --check
• Pre-push hook: typecheck + build:lib

Fixes #1988

[aao-ipr-bot]

Approving. Closes the v5 raw-response-builder write-only-field leak it targets and aligns ServerPayload<T> with the runtime contract — spec-conformant per core.generated.ts:997, 1072, 1104, 1123, fail-closed at the seam.

Things I checked

• BusinessEntity.bank and NotificationConfig.authentication.credentials are normatively write-only in the spec (generated-type docstrings cite the schema directly). Stripping them at the v5 raw builders is enforcing a MUST NOT be echoed contract, not witness violation.
• Runtime helpers in src/lib/server/responses.ts:160-206 mirror the v6 projectBillingEntity / projectNotificationConfig prior art at src/lib/server/decisioning/account.ts:884-942. Same spread-clone-then-strip shape, non-mutating by inspection.
• StripWriteOnlyResponseFields<T> evaluation order in src/lib/types/server-payload.ts:43-55 is correct: recursive map → strip bank → strip authentication.credentials. JsonPrimitive distribution works. Readonly-array arm is unreachable because mutable arrays match the earlier (infer TItem)[] arm.
• Five v5 builders touched (mediaBuyResponse, updateMediaBuyResponse, getMediaBuysResponse, listAccountsResponse, syncAccountsResponse) cover every wire response shape that carries BusinessEntity or nested account.notification_configs on the v5 path.
• Sync-arm idempotency cache stores formatted (post-strip) at create-adcp-server.ts:4806-4822, so the replay path is clean. Test name "before idempotent replay caching" is correctly placed.
• Signature narrowings on AccountStore (upsert, syncGovernance, list, reportUsage, getAccountFinancials) and the brand-rights / sales specialism methods are backward-compat under structural subtyping — extra fields in the adopter's return value remain assignable to the narrower *Payload type.
• New runtime tests in test/server-responses.test.js and test/lib/account-notification-config-projection.test.js exercise the new strip paths at all four affected v5 builders, including nested account.billing_entity.bank inside media buys.

Follow-ups (non-blocking — file as issues before the next beta cut)

1. v6 HITL tasks/get + completion-webhook side-channel echoes unstripped result. from-platform.ts:2571-2574 (HITL completion strip) and :1895-1897 (tasks/get echo) bypass mediaBuyResponse entirely. An adopter returning CreateMediaBuySuccess from ctx.handoffToTask(...) with account.billing_entity.bank or notification_configs.authentication.credentials leaks both to the polling buyer AND the completion webhook receiver. Same chokepoint pattern PR #1562 added for ctx_metadata / implementation_config — bank / credentials need the same treatment. Fix: add a stripWriteOnlyResponseFields walk in ctx-metadata/wire-shape.ts mirroring stripCtxMetadata's carrier traversal, invoke at from-platform.ts:2289-2292 (projectSync) and :2571-2574 (HITL completion). That single addition closes this AND #2 below.
2. Sync-arm emitSyncCompletionWebhook reads pre-wrap() projected payload. from-platform.ts:2459 — when autoEmitCompletionWebhooks: true, the webhook receiver gets the unstripped payload before the dispatcher's wrap strips it. Closed by the same projectSync strip in follow-up #1.
3. ctxMetadataStore.setResource persists unstripped resources at rest. from-platform.ts:2870 via autoStoreResources. At-rest exposure in the configurable Postgres/Redis backend. Lightweight fix: reuse the wire-shape walk in autoStoreResources at :2860-2864 before store.setResource.
4. Tighten StripBusinessEntityBank<T> predicate. server-payload.ts:38 matches any object with a top-level bank?: — today only BusinessEntity qualifies, but a future schema landing that adds bank?: anywhere else (or an adopter using ServerPayload<MyType> with an unrelated bank field) silently drops it with no diagnostic. The runtime strip in responses.ts:160 only targets billing_entity / invoice_recipient parents — types and runtime diverge. Either narrow the type predicate to { legal_name: string; bank?: unknown } or walk by key name (mirror runtime).
5. Changeset doesn't disclose the ServerPayload<T> recursive narrowing. The body documents the *Payload / *HandlerResult alias swaps, not the fact that every signature now returning *Payload will reject bank / authentication.credentials at compile time. Adopters mid-migration who spread an incoming request into a response will hit fresh TS errors. Worth a line in the changeset.

Minor nits (non-blocking)

1. let stripped should be const. src/lib/server/responses.ts:189 — never reassigned. @typescript-eslint/prefer-const will flag.
2. BuildCreativeReturn and BuildCreativePayload duplicated across creative.ts and creative-ad-server.ts. Identical bodies pointing at the same generated BuildCreativeSuccess / BuildCreativeMultiSuccess. The index aliases the creative-ad-server variants, so no .d.ts duplicate-symbol risk today. Silent-drift risk if one is modified without the other. Consider extracting to a shared decisioning/creative-shared.ts.
3. ListAccountsPayload vs ListAccountsHandlerResult naming collision. account.ts:233-246 — same domain, different shape (wire-stripped { accounts: [...] } vs framework rich CursorPage<Account<TCtxMeta>>). Worth a 2-line tsdoc on each distinguishing handler-layer from wire-payload.
4. @ts-expect-error in check-adopter-types.ts:193-202 can silently skip the assertion. If accounts[0]?.notification_configs?.[0] is undefined the guard short-circuits and the type check never runs against a populated value. Use non-null assertions so the type check actually exercises the strip.
5. brand-rights.ts:138, 165-175 TSDoc lag. Doc above acquireRights still names the three concrete AcquireRights* arms; signature now returns AcquireRightsPayload. Same for updateRights. Doc-vs-signature drift, easy fix.

Notable timing aside: the changeset prose was updated in this PR (file modified, not added) while the version got bumped to beta.9 in package-lock.json and version.ts without a corresponding package.json change in the diff — read as the regen catching up after a base-branch release, not a manual bump. Worth a glance before merge to confirm package.json is consistent with what npm will see.

Safe to merge once the v6 follow-ups (#1 + #2 above) get tracked as issues. Spec-conformance plus the v5-path coverage carry the approval.

[aao-ipr-bot]

LGTM. Type-level and v5-runtime stripping are now symmetric. Follow-ups below — the most important is the v6 framework path parity.

Things I checked

• Type recursion in src/lib/types/server-payload.ts:33-71. StripWriteOnlyResponseFields<T> distributes correctly over T extends unknown (preserves discriminated unions — AcquireRightsPayload still discriminates on rights_status, CreativeApprovalPayload on approval_status, both renamed from status in 3.1 per adcp#4878), recurses through arrays and readonly arrays, and bottoms out at JsonPrimitive. Bounded by the depth of generated types (4-6 levels max on GetProductsResponse / GetMediaBuysResponse); proven under --noEmit by decisioning.type-checks.ts:436-484.
• Runtime symmetry on v5. stripBusinessEntityBank (responses.ts:160-172), stripAccountWriteOnlyFields (:174-176), stripMediaBuyWriteOnlyFields (:188-196), and stripMediaBuysWriteOnlyFields (:198-206) cover listAccountsResponse / syncAccountsResponse / mediaBuyResponse / updateMediaBuyResponse / getMediaBuysResponse. The destructure-and-rest idiom for bank matches the existing projectBillingEntity precedent at account.ts:938-942 — excludes own-enumerable, prototype-chain, and Proxy-backed bank fields by ES rest-spread evaluation order.
• Spec authority. BusinessEntity.bank write-only at src/lib/types/v3-1-beta/tools.generated.ts:12767, 12821 ("MUST NOT be echoed in responses"); NotificationConfig.authentication.credentials write-only at :12500, 12848, 12864 ("sellers MUST NOT echo"). The PR's stripping is normatively required, not a stylistic choice.
• Changeset present. .changeset/fix-server-handler-payload-aliases.md at patch. Defensible on the 8.1.0-beta.x line; would be minor on the GA line because the new alias exports are additive AND ServerPayload<T> narrows existing types. Migration prose in the changeset body covers the adopter switch.
• Version sync. LIBRARY_VERSION 8.1.0-beta.8 → beta.9 in version.ts + package-lock.json is a catch-up to package.json which was bumped by release PR #1980. Not a manual cut.
• Test coverage. New cases in test/server-responses.test.js:163-194, 272-282, 308-345 and test/lib/account-notification-config-projection.test.js:73-138 cover all five hardened response builders. 5473/5473 pass after the rebase, per the green CI on the fix: address ci failures commit.

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

• v6 framework path doesn't runtime-strip invoice_recipient.bank. runtime/from-platform.ts:3658, 3750, 3878 returns the platform result raw from createMediaBuy / updateMediaBuy / getMediaBuys. TypeScript adopters are protected by Promise<CreateMediaBuyPayload> at the seam; JS adopters and as-cast paths still leak bank coordinates onto the wire AND into the idempotency replay cache — exactly the threat the comment at account.ts:899-908 names for governance_agents.authentication.credentials. Add a toWireMediaBuy(...) projection mirroring toWireAccount (account.ts:804-840) and apply it in the three v6 paths. stripMediaBuyWriteOnlyFields in responses.ts:188-196 is the right model — factor it out. This PR fixed the v5 deprecated path; v6 parity is the next ticket.
• patch vs minor on the GA line. Document the ServerPayload<T> narrowing explicitly in the changeset body so the GA-cut release-note generator picks it up. Adopters whose handlers currently set invoice_recipient.bank (silly but legal pre-PR) will fail typecheck on bump.
• Conformance assertion for write-only absence on v6. decisioning.type-checks.ts:507-565 only proves compile-time. A storyboard step asserting bank / credentials absence in the dispatched wire on create_media_buy / update_media_buy / get_media_buys would close the runtime gap above and prevent regression after the toWireMediaBuy follow-up lands.

Minor nits (non-blocking)

1. JsonPrimitive ordering. src/lib/types/server-payload.ts:33 includes undefined. Works in practice — the mapped type evaluates first — but T extends string | number | boolean | null would make the conditional ordering load-bearing on shape, not optionality. Tighten it.
2. StripAuthenticationCredentials over-matches by shape, not name. src/lib/types/server-payload.ts:35-39 strips .credentials from ANY object with a typed authentication?: object property. Today only NotificationConfig and PushNotificationConfig qualify, and both are write-only. If a future schema adds an unrelated authentication: { credentials } (e.g., a brand-rights generation_credentials echo block — responses.ts:768 hints at one), this strips it silently. Document the assumption inline.
3. CreateMediaBuyHandlerResult = CreateMediaBuyPayload | TaskHandoff<CreateMediaBuyPayload>. Wrapping ServerPayload<T> inside TaskHandoff<T> is correct — the deferred value is also wire-shaped, so the strip applies once at the framework dispatch boundary. Worth a one-line comment so future readers don't strip-twice. sales.ts:115-116.

Approving on the strength of the spec-pinned write-only contract being met on the v5 path plus the type-level guarantee at the v6 seam. v6 runtime parity is the third drift-cleanup ticket in this area — file it before the next adopter ships invoice_recipient: { bank: db.row.bank } from a JS handler.

[aao-ipr-bot]

LGTM. Fixes #1988 with the right shape — named *Payload / *HandlerResult aliases that drop the protocol envelope so adopter helper layers wrapping Result<Payload, E> stop being forced to hand-stamp status: TaskStatus. The bonus hardening — recursive write-only stripping in ServerPayload<T> plus the raw v5 builders — is the right shape too: the spec calls these MUST-NOT-echo fields (core.generated.ts:704,997,1123), so stripping is contract enforcement, not translation.

Things I checked

• The new StripWriteOnlyResponseFields<T> in src/lib/types/server-payload.ts:43-55 only collides with two bank?: sites across the entire generated graph (core.generated.ts:1074, tools.generated.ts:8157) — both BusinessEntity. Same for authentication?: — only NotificationConfig is reachable from response shapes; the required-authentication sites (ReportingWebhook, artifact_webhook, push_notification_config) are request-side or already envelope-stripped. The optionality widening on authentication?: is therefore harmless today.
• Runtime strip helpers in src/lib/server/responses.ts:160-206 walk billing_entity, invoice_recipient, and nested account.* correctly. Control flow on stripMediaBuyWriteOnlyFields:188-196 traced — early-returns the bank-stripped result when account is absent; no silent data loss.
• The // @ts-expect-error regression alarms in decisioning.type-checks.ts:529,534,558,563 and scripts/check-adopter-types.ts:228,233 are robust — if the write-only fields come back, the access compiles, the directive becomes unused, and tsc --noEmit fails loudly.
• Tests in test/server-responses.test.js:164-194,273-283,309-344 and test/lib/account-notification-config-projection.test.js assert behavior ('bank' in result.structuredContent…), not implementation. Right shape.
• Changeset exists at .changeset/fix-server-handler-payload-aliases.md.

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

1. v6 framework dispatch path leaks invoice_recipient.bank and nested account.* write-only fields. createAdcpServerFromPlatform's createMediaBuy / updateMediaBuy / getMediaBuys / listCreatives / syncCreatives dispatch through projectSync(fn, r => r) at src/lib/server/decisioning/runtime/from-platform.ts:3651,3745,3796,3873,3928. projectSync only strips ctx_metadata + implementation_config (from-platform.ts:2289-2292). An adopter who spreads a DB row (or as-casts) into the platform return ships IBAN to the buyer wire and the idempotency replay cache. The type strip catches strictly-typed returns; loose paths bypass it. Pre-existing — this PR doesn't introduce the regression — but the natural follow-up is a toWireMediaBuy projector mirroring toWireAccount (account.ts:804-882), reusing the new helpers at the wire-emit boundary. Same shape toWireSyncGovernanceRow already uses.

2. listCreativesResponse / syncCreativesResponse v5 raw builders also leak. ListCreativesResponse.creatives[].account (tools.generated.ts:13609) and SyncCreativesSuccess.creatives[].account (tools.generated.ts:13945) carry the same billing_entity.bank + notification_configs[].authentication.credentials surfaces stripped on listAccountsResponse / getMediaBuysResponse. Builders at responses.ts:462,570 pass data through verbatim. One-line extension: stripCreativesWriteOnlyFields mirroring stripMediaBuysWriteOnlyFields. Squarely in this PR's stated scope; landed for the highest-traffic paths but missed these two.

3. Changeset patch vs minor. ServerPayload<T> is a public exported type and now strips bank / credentials from nested objects. Any adopter currently typing ServerPayload<ListAccountsResponse>['accounts'][n]['billing_entity']['bank'] gets a fresh type error. Defensible as a bug fix aligning the type with the wire's MUST-NOT-echo contract, but minor is the more honest bump for the public-type surface change. Alternatively, call the type narrowing out explicitly in the changeset body so it surfaces in release notes.

4. Handler-result alias contract asymmetry. CreateMediaBuyHandlerResult / SyncCreativesHandlerResult carry Payload | TaskHandoff<Payload> (sales.ts:116-117) — real semantic content. ReportUsageHandlerResult = ReportUsagePayload and GetAccountFinancialsHandlerResult = GetAccountFinancialsSuccessPayload (account.ts:58-59) are trivial 1:1 re-exports. If reportUsage ever gains handoff support, adopters who annotated handlers as ReportUsagePayload would silently keep compiling against a wider return type. Either add a one-line comment noting the trivial-alias status, or unify by giving every handler the Payload | TaskHandoff<Payload> shape on day one.

Minor nits (non-blocking)

1. let stripped is never reassigned at src/lib/server/responses.ts:188 — const. Control flow checked; correct.
2. Document the StripBusinessEntityBank<T> assumption at src/lib/types/server-payload.ts:41. Today only BusinessEntity has bank?:; future schema additions could collide. Either tighten to T extends BusinessEntityLike or drop a one-line comment alongside StripAuthenticationCredentials explaining the codegen-audit invariant.
3. Note the authentication?: optionality widening rationale alongside StripAuthenticationCredentials at server-payload.ts:35-39. Safe today (no response type has a required authentication block), but the audit is non-obvious and worth pinning in source.

Safe to merge. Follow-up issues for items 1 and 2 above should land before the next release if any v5 raw-handler adopters remain on listCreatives / syncCreatives, and item 1 (v6 framework gap) is the bigger spec-compliance win and worth its own PR soon.