feat: enforce buyer-agent billing gates

[bokelley]

Summary

Closes #1292.

• enforce sync_accounts.billing against seller-wide supportedBillings and resolved BuyerAgent.billing_capabilities
• return clamped per-account billing failures for BILLING_NOT_SUPPORTED, BILLING_NOT_PERMITTED_FOR_AGENT, PAYMENT_TERMS_NOT_SUPPORTED, and BRAND_REQUIRED
• add BuyerAgentRegistry.suggestBilling() and DecisioningCapabilities.supportedPaymentTerms
• avoid idempotency replay caching for rejected sync_accounts commercial rows so onboarding/capability changes can take effect on retry
• document the AdCP 3.1 buyer-agent billing gate behavior and update stale registry guidance

Expert Review

• DX/docs review found stale Phase 2 wording and missing behavior notes; addressed before opening
• JavaScript/protocol and code-review follow-up agents did not return before PR creation; local validation covers the touched dispatch and idempotency paths

Validation

• npm run typecheck
• npm run build:lib
• npm run format:check
• NODE_ENV=test node --test --test-timeout=60000 test/server-buyer-agent-billing.test.js test/server-account-store-ctx-forwarding.test.js test/server-idempotency.test.js

[aao-ipr-bot]

LGTM. Follow-ups noted below. Framework-level commercial enforcement on sync_accounts lands the AdCP 3.1 buyer-agent billing gate cleanly, and the per-row vs. request-level split is forced by the schema's row-key requirement, not invented.

Things I checked

• Order-preserving merge in src/lib/server/decisioning/runtime/from-platform.ts:5047-5063. Walked all-accepted, all-rejected, and [failed, accepted, failed, accepted] mixed cases — acceptedIndex increments only on the accepted branch and never overruns rows[]. The empty-accepted short-circuit Promise.resolve([]) at L5046 keeps accounts.upsert from being called with [] when policy rejects everything.
• Error-code conformance against the SDK's pinned 3.1.0-beta.5 schema. All four codes (BILLING_NOT_SUPPORTED, BILLING_NOT_PERMITTED_FOR_AGENT, PAYMENT_TERMS_NOT_SUPPORTED, BRAND_REQUIRED) are in ErrorCodeValues at src/lib/types/enums.generated.ts:77, and the details shapes match BillingNotPermittedForAgentDetailsSchema / BillingNotSupportedDetailsSchema at schemas.generated.ts:5007-5015. The clamped single-suggested_billing echo is spec-canonical, not an SDK opinion.
• BRAND_REQUIRED whole-request throw. The schema requires brand+operator non-optional on row shape (tools.generated.ts:21505-21580) — there is no way to emit a per-row rejection without a row key. The request-level throw at from-platform.ts:4965 is forced, not chosen.
• Idempotency cache bypass at create-adcp-server.ts:2013-2050. hasUncacheableSyncAccountRejection gates only mutations under idempotencyCheck && idempotency — list_accounts and other read paths don't reach this code. IDEMPOTENCY_IN_FLIGHT collapses concurrent same-key retries before the gate re-executes.
• Frozen BuyerAgent invariant. New code (enforceSyncAccountsCommercialPolicy, suggestBilling, buildBillingNotPermittedError) only reads agent.billing_capabilities via .has() and never enters the wire shape. Object.isFrozen guard at L3621 stays intact.
• Changeset. minor is correct: additive surface (suggestBilling, supportedPaymentTerms), gated behavior change (only fires when agentRegistry is configured), no exported-symbol removal or required-flip.

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

• Onboarding-oracle closure is partial. The docstring at create-adcp-server.ts:1387 claims unmapped bearers get the "oracle-safe BILLING_NOT_SUPPORTED surface rather than BILLING_NOT_PERMITTED_FOR_AGENT." That closes the details-shape leak but opens an error-code distinguisher: unmapped bearer → BILLING_NOT_SUPPORTED, mapped-but-no-capability → BILLING_NOT_PERMITTED_FOR_AGENT. An attacker who knows the seller's default supportedBillings can probe a credential and learn whether it's provisioned in the registry without changing state. The mirror pattern at tools.generated.ts:25827-25831 (AgentPermissionDenied) is consistent — so this is the canonical clamp shape, not a regression — but the docstring overclaims the closure. Either tighten the doc to "prevents details.supported_billing leakage to unmapped probes" or unify the unmapped-bearer path to PERMISSION_DENIED. Spec follow-up worth filing against adcp#3831 if the protocol intent is to fully close credential-existence enumeration.
• Idempotency-cache bypass is spec-silent SDK policy. AdCP 3.0 GA's idempotency_key contract is same-key→same-response within TTL. UNCACHEABLE_SYNC_ACCOUNT_ERROR_CODES deliberately breaks that contract so capability changes take effect on retry — defensible, but cross-SDK adopters on other runtimes will replay the old rejection. File a sibling spec PR alongside adcp#3831 codifying the carve-out, or note in the migration guide that this is @adcp/sdk-specific behavior.
• supportedBillingsFor defaults to ['agent'] when omitted. Inconsistent with supportedPaymentTerms' "omit → defer to accounts.upsert" philosophy in the new docstring at capabilities.ts:236-241. Either align (omit → no framework gate) or document the asymmetric default. Surprises operator-billed retail-media sellers who don't set the field.
• Test coverage gaps. Every test ships exactly one account, so the order-preserving merge has zero behavioral coverage. Add [rejected, accepted, rejected, accepted] with assertions on positions + captures.upsertCalls === 1 carrying only the accepted refs. Also missing: agentRegistry undefined entirely, non-string entry.billing, the entry.account.brand duck-typed branch of entryBrandOperator.
• Adopter observability on framework rejections. accounts.upsert never sees the entries the framework rejected — adopters can't log, audit, or override. Consider a onCommercialRejection?(rejectedRows, ctx) hook before shipping the v8 surface freeze.

Minor nits (non-blocking)

1. Field-path inconsistency. from-platform.ts:4927,4942 emit field: 'accounts[].billing' (no index) while payment-terms at L5000 and BRAND_REQUIRED at L4968 emit accounts[${index}]. Thread index through buildBillingNotSupportedError / buildBillingNotPermittedError for buyer DX symmetry.
2. toWireSyncAccountRow call sites duplicated. L5049 (early return) and L5063 (merged branch) both combined.map(toWireSyncAccountRow). Future projection bug fixes have to land twice. Cosmetic — collapse once originalCount === acceptedCount is the only fast-path optimization.
3. BILLING_VALUES duplicates spec enum at L4861. Import the value-list from tools.generated.ts if exported, otherwise drop a // keep in sync with BillingParty pointer.
4. Phase-2 cleanup pass is finally complete — the docs noted the gate was coming for three releases. Stale "Phase 2 lands in #1292" wording is gone in this PR. Worth the audit.

Approving on the strength of the schema conformance plus the forced row-key shape — every wire surface validates and the request-level throw is the only schema-conformant escape hatch. Land the multi-entry mixed-merge test in a follow-up; the implementation is correct on paper.

[aao-ipr-bot]

Solid. Right shape — gate fires only when agentRegistry !== undefined, so opt-in adopters get enforcement and everyone else sees no behavior change. code-reviewer, security-reviewer, and ad-tech-protocol-expert ran in parallel; protocol verdict was sound-with-caveats against adcontextprotocol/adcp#3831. Three real follow-ups below — one production-relevant, the others design/coverage. PR description noted the protocol and code-review agents didn't return before opening; the items below are roughly what they would have caught.

Things I checked

• Changeset present, minor is correct — behavior change is opt-in via platform.agentRegistry, no existing adopter shifts.
• BILLING_NOT_SUPPORTED (unmapped bearer) omits details.scope — matches the normative "MUST omit" rule in adcp#3831's billing-not-supported.json.
• BILLING_NOT_PERMITTED_FOR_AGENT.details is { rejected_billing, suggested_billing? } with no permitted_billing leak (additionalProperties: false-compliant). Test at L156-176 asserts the negative explicitly. Good.
• suggestBilling returns single value, operator > advertiser > agent — spec-aligned commercial fallback; never the agent's full permitted set.
• failedSyncAccountRow rebuild path covers field/suggestion/retry_after/details from AdcpStructuredError — no field loss on the throw branch.
• Original params.accounts array is not mutated; acceptedParams = { ...params, accounts: acceptedEntries } is a shallow clone with a fresh array.

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

1. Production-relevant: isPaymentTerms is typeof value === 'string' (from-platform.ts:4867). The value lands verbatim in the response envelope at L4998: \Payment terms "${entry.payment_terms}" are not supported...`. With validation: 'off'an attacker-controlledpayment_termsstring flows into wire responses, server logs, and any LLM downstream of the sync result. Perdocs/guides/CTX-METADATA-SAFETY.md, wire-strip doesn't protect log lines or LLM context. The schema defines a closed enum — narrow the guard. Same pattern applies to any future error-message echo of buyer-supplied entry.*` fields.

2. supportedBillingsFor default of ['agent'] (from-platform.ts:4871-4874). When capabilities.supportedBillings is undefined or empty, the helper defaults to ['agent'] and rejects operator/advertiser. The reference adapter at examples/hello_seller_adapter_guaranteed.ts:528-532 had to add supportedBillings: ['operator', 'agent'] specifically because of this — that's the tell. Compare against supportedPaymentTerms's "omit to leave validation to accounts.upsert" doc; the seller-wide billing capability should default the same way. Otherwise every adopter who upgrades silently rejects valid traffic.

3. BRAND_REQUIRED aborts the entire batch (from-platform.ts:4964-4970). Every other gate emits a per-row failure via failedRows.set(index, …); BRAND_REQUIRED throws an envelope error that kills the whole sync_accounts call. A buyer batching ten accounts where one is malformed gets zero rows processed instead of nine accepted + one rejected. Inconsistent failure-blast-radius. Spec doesn't pin batch semantics here, but per-row is what the rest of the policy already does.

4. Idempotency partial-accept re-executes accounts.upsert on previously-accepted rows. When hasUncacheableSyncAccountRejection returns true (any row carries one of the three codes), the entire response bypasses the cache (create-adcp-server.ts:2040-2042). On retry with the same key, policy re-runs and accepted entries hit accounts.upsert again. Adopter upsert is implicitly expected to be row-level idempotent — that contract isn't stated in BUILD-AN-AGENT.md. Either cache the combined response (rejected rows replay verbatim, buyer uses a new key after capability upgrade) or document the adopter contract loudly.

5. hasUncacheableSyncAccountRejection short-circuits on adopter rejections without errors[] (create-adcp-server.ts:2032: return record.status === 'rejected' when errors is not an array). Any adopter-returned {status:'rejected'} row without an errors[] field bypasses the cache, even for stable rejections (credit hold, brand suspended). Either require errors[] and code-match, or document the broader cache-bypass policy in the comment at L4870-4874.

6. Test coverage gap — merge-back ordering is never exercised with mixed batches. All seven cases in test/server-buyer-agent-billing.test.js pass accounts: [account]. The for (let index = 0; index < originalCount; index += 1) interleaving loop at L5052-5062 only runs with acceptedIndex=0 or failedRows.size=0. Add a [good, bad, good] case asserting positions are stable — the load-bearing behavior the PR claims to enable.

Minor nits (non-blocking)

1. field pointer indexing is inconsistent. BILLING_NOT_SUPPORTED and BILLING_NOT_PERMITTED_FOR_AGENT use accounts[].billing (no index); PAYMENT_TERMS_NOT_SUPPORTED and BRAND_REQUIRED use accounts[i].payment_terms / accounts[i].brand. adcp#3831 doesn't normatively pin the convention so neither form is wire-wrong, but the inconsistency in a single response is a defect for agents reconciling per-row failures. Recommend indexing all three. from-platform.ts:4927, 4942 vs. 4969, 5000.

2. Self-introspection of bearer mapping. BILLING_NOT_PERMITTED_FOR_AGENT is observationally distinct from the unmapped-bearer BILLING_NOT_SUPPORTED surface — an authenticated buyer can probe their own bearer's mapping state. This is the spec's design intent in adcp#3831, not a SDK bug, and the unmapped-vs-seller-wide oracle that the PR explicitly closes is closed correctly. Flagging only because the docstring at create-adcp-server.ts:1387-1389 calls the path "oracle-safe" unqualified — the framing applies only to the unmapped vs seller-wide-capability case, not to self-introspection.

LGTM. Follow-ups noted below.

[aao-ipr-bot]

Right shape on the commercial gate and the oracle protection. Two concrete defects in the same enforceSyncAccountsCommercialPolicy function fail closed in the wrong direction — both reachable from realistic adopter flows.

Must fix

1. isPaymentTerms is unbounded — src/lib/server/decisioning/runtime/from-platform.ts:4867-4869. Returns true for any string. PaymentTerms in the generated types is the closed enum 'net_15' | 'net_30' | 'net_45' | 'net_60' | 'net_90' | 'prepay'. Two failure modes downstream:

   • When supportedPaymentTerms is unset (line 4992-4994), the entry skips the gate entirely and accounts.upsert receives the junk value. Framework gate is fail-open on its own contract.
   • When supportedPaymentTerms is set, the attacker string is interpolated into errors[0].message at line 4998 verbatim with no charset or length cap. Log-poisoning surface on every adopter that ships structured-error logging.

   Mirror the closed-enum membership pattern from isBillingParty / BILLING_VALUES at lines 4861-4865.

2. account_id-only billing rejection collapses the whole batch — from-platform.ts:4898-4909. Walk the flow for { account: { account_id: 'X' }, billing: 'agent' }:

   • entryHasAccountId returns true (line 4894), so the BRAND_REQUIRED guard at 4964 does NOT fire.
   • failure is assigned BILLING_NOT_PERMITTED_FOR_AGENT (or BILLING_NOT_SUPPORTED).
   • failedSyncAccountRow runs entryBrandOperator(entry) — returns undefined.
   • Rethrows as a top-level AdcpError, destroying every sibling accepted row in the same batch.

   The synthesized row at lines 4910-4916 needs brand + operator to anchor against. Either widen SyncAccountsResultRow.brand / operator to optional when account_id is present and emit a row-level rejection, or reject account_id-only billable entries up front with a top-level error before partitioning. Test gap below masked this — every test wraps a single entry in accounts: [account], so the projector's index-alignment loop (lines 5051-5062) is unexercised on mixed batches.

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

• Multi-entry batch coverage in test/server-buyer-agent-billing.test.js. The projector at 5051-5062 is the most fragile new code in the PR. Add: (a) accepted + rejected mixed batches verifying ordering preservation, (b) account_id-only billing-rejection entries (the must-fix #2 above), (c) IDEMPOTENCY_CONFLICT for same key + different accounts[] after the policy.acceptedParams rewrite at line 5039-5041.
• BRAND_REQUIRED envelope-level throw at 4965-4969 diverges from the per-row failed pattern used everywhere else in the policy. ad-tech-protocol-expert flagged this as an open question against adcp#3831 — confirm whether the spec registers BRAND_REQUIRED as top-level or per-row before locking the behavior.
• supportedBillings defaults to ['agent'] at 4871-4874. Silent narrowing for any 3.0.x adopter that wired agentRegistry without declaring supportedBillings and is live with operator- or advertiser-billed relationships. Either bootstrap-warn when agentRegistry is set and supportedBillings is unset, or call this out in docs/migration-buyer-agent-registry.md § Billing Enforcement.
• dispatchCtx rewrites ctx.input.accounts at 5039-5041 to the filtered slice. Contract change for adopter accounts.upsert implementations reading ctx.input. Worth a doc line.
• Idempotency canonical-payload digest — if the framework hashes post-rewrite params for IDEMPOTENCY_CONFLICT detection, a retry where the buyer fixes only a rejected row will canonical-diff and trip a spurious conflict. Confirm the digest is computed on pre-rewrite params.

Things I checked

• Changeset present, minor bump matches the additive surface (.changeset/billing-agent-gates.md). Type widening on SyncAccountsResultRow.errors[] is structurally non-breaking — {code, message} literals still satisfy the new SyncAccountError.
• AdCP 3.1 error-code shapes match adcp#3831: BILLING_NOT_PERMITTED_FOR_AGENT.details = {rejected_billing, suggested_billing?}, BILLING_NOT_SUPPORTED.details = {scope?, supported_billing?}, suggested_billing singular (NOT permitted_billing[]), recovery: 'correctable'. core.generated.ts:25862-25879 confirms additionalProperties: false.
• Oracle protection at from-platform.ts:4980-4984 omits details.scope on the unmapped-bearer path. Per ad-tech-protocol-expert, this is the normative shape from adcp#3831.
• buildBillingNotPermittedError value reflection is bounded — requestedBilling clears isBillingParty (closed enum) before reaching the error builder. No injection vector on the billing path (only the payment_terms path above).
• hasUncacheableSyncAccountRejection predicate at create-adcp-server.ts:2022-2034 walks the right structuredContent path and conservatively falls back to status === 'rejected' when errors[] is absent.
• suggestBilling order (operator > advertiser > agent) matches the spec's commercial fallback discussion.

Minor nit

1. Inconsistent field indexing. from-platform.ts:4927 uses accounts[].billing (non-indexed) but line 5000 uses accounts[${index}].payment_terms (indexed). Pick one.

Request changes for the two must-fix items. Happy path and oracle-protection shape are right — these are bounded fixes.

[aao-ipr-bot]

LGTM. The SDK enforces framework-level commercial policy at the declared-capability boundary; adopters never see rows the seller can't sell, and the wire shape matches adcp#3831 exactly.

Things I checked

• Generated details shapes match: BillingNotSupportedDetails (src/lib/types/core.generated.ts:25871) and BillingNotPermittedForAgentDetails (core.generated.ts:25861). scope, supported_billing, rejected_billing, suggested_billing are all schema-defined — none invented.
• Oracle-safety: from-platform.ts:4980-4984 returns BILLING_NOT_SUPPORTED with empty details for unmapped-bearer requests under a configured registry. Matches manifest.generated.ts:107 ("MUST receive BILLING_NOT_SUPPORTED instead… per-agent code or account-scope hint without established identity is a cross-tenant onboarding oracle"). Correct.
• Witness-not-translator holds: framework dispatches policy.acceptedEntries to accounts.upsert; failed rows reconstructed positionally at projection (from-platform.ts:5051-5063). Adopter never sees fabricated rows.
• Idempotency claim/release symmetry: cache-write gate (create-adcp-server.ts:4887) and release-on-no-cache (:4915) both pivot on shouldCacheIdempotencyResponse. No path leaves a claim held without caching.
• Changeset present (minor), matches the additive surface (supportedPaymentTerms, suggestBilling).
• Tests cover all four codes + the replay-bypass scenario across a capability widening.

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

1. Combined-row merge has no length check. from-platform.ts:5059 casts rows[acceptedIndex] as SyncAccountsResultRow with no assertion that rows.length === policy.acceptedEntries.length. Adopter returning fewer rows promotes partial-success to a 500 via toWireSyncAccountRow(undefined). Assert up front and emit a SERVICE_UNAVAILABLE with a "handler returned N rows for M requested" diagnostic — contract violation the buyer can't fix, framework should surface loudly.
2. Replay-protection coverage gap. create-adcp-server.ts:2030-2032 returns record.status === 'rejected' when errors is not an array, treating malformed adopter rejection rows as uncacheable. An adopter emitting {status:'rejected', action:'failed'} without errors[] (transient downstream outage shape) defeats replay caching for that response — every retry re-executes the upstream side-effect. Invert the fallback (non-array errors → cacheable) and add a regression test. Security-reviewer Medium; worth fixing today.
3. BRAND_REQUIRED is request-level; every other gate is per-row. from-platform.ts:4965-4970 throws and aborts the whole batch when one billable entry lacks brand/operator/account_id. manifest.generated.ts:121-124 describes BRAND_REQUIRED with field: accounts[N].brand — per-row positioning. Convert to per-row using row index as the synthetic key. Same fix covers the related re-throw at failedSyncAccountRow (from-platform.ts:4898-4909): an account_id-only entry that passes BRAND_REQUIRED but fails billing gets promoted to request-level because entryBrandOperator returns undefined. Stamp synthetic brand/operator from the resolved Account, or widen SyncAccountsResultRow to allow account_id-only failed shape. No test covers this path.
4. suggestBilling order docstring overstates the spec. buyer-agent.ts:209 claims "spec-discussed commercial fallback order: operator > advertiser > agent." adcp#3831 only normatively pins "typically operator." advertiser > agent after operator is an SDK opinion. Soften the docstring to "SDK convention; adopters can wrap" or raise the ordering with the spec WG.
5. Idempotency bypass for the three commercial codes is undocumented at the spec level. CLAUDE.md's protocol-wide rule says "Handlers must return the same response when the same key is replayed." Defensible for framework-gated rejections (the handler never ran the first time), but file an issue against adcontextprotocol/adcp to pin the semantics — otherwise the bypass is SDK opinion that's hard for adopters to anticipate.

Minor nits (non-blocking)

1. isPaymentTerms predicate is a lie. from-platform.ts:4867 returns value is PaymentTerms from a bare typeof === 'string' check. Narrow against a literal union derived from PaymentTerms, or rename to isStringPaymentTermsCandidate.
2. supportedBillings default is hidden. from-platform.ts:4873 falls back to ['agent'] when the platform omits supportedBillings (or sets it empty). capabilities.ts:233 docstring doesn't mention this — easy to miss when wiring a new platform. One line in the docstring.

LGTM. File Follow-ups 1 and 2 today.