feat(training-agent): implement BILLING_NOT_SUPPORTED + BILLING_NOT_PERMITTED_FOR_AGENT gates#3851
Merged
Merged
Conversation
…ERMITTED_FOR_AGENT gates Reference implementation of the two billing gates from #3831 (BuyerAgentRegistry spec backing). Lands first on the legacy /mcp route via handleSyncAccounts; v6 per-tenant routes have no accounts.upsert yet, so wiring there follows in a separate PR. Capability gate: validates input.billing against SUPPORTED_BILLINGS and emits BILLING_NOT_SUPPORTED with error.details.scope: "capability" and an echoed supported_billing list per error-details/billing-not-supported.json. Also fixes the stale advertisement (was 'agent'-only, handler accepted all three) so get_adcp_capabilities and the gate agree. Per-buyer-agent gate: new commercial-relationships.ts module maps the authenticated principal to a commercial-relationship enum (passthrough_only or agent_billable). Two demo bearer prefixes recognized — demo-billing-passthrough-* and demo-billing-agent-billable-* — matching the existing DEMO_TEST_KIT_KEY_PATTERN authenticator. Emits BILLING_NOT_PERMITTED_FOR_AGENT with the clamped details shape per error-details/billing-not-permitted-for-agent.json (additionalProperties: false — rejected_billing + suggested_billing only). Strict toEqual assertion in the test enforces the clamp at the wire level. Bright-line uniform-response rule: principals without an onboarded record return undefined from getCommercialRelationship and fall through to the seller-wide capability gate only — preventing the per-agent code from acting as an onboarding oracle for unrecognized callers. tenantId reserved on getCommercialRelationship signature so the v6 follow-up doesn't force a signature break across call sites. 20 unit tests pass (14 existing + 6 new): capability-gate rejection, passthrough rejection of agent and advertiser, autonomous recovery via operator, agent-billable acceptance of all three values, uniform-response for unrecognized principal, and dry_run does not suppress billing-gate rejections. Reviewed by code-reviewer; SHOULD FIX (tenantId plumbing) and both NICE TO HAVE items (toEqual clamp assertion, dry_run test) addressed. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…, multi-account batching test Addresses SHOULD FIX findings from second-pass review of #3851: - Export SUPPORTED_BILLINGS from account-handlers.ts and import it in task-handlers.ts so the gate constant and the supported_billing advertisement are mechanically locked rather than comment-coupled. - Document the security posture of the static:demo:* principal namespace explicitly in commercial-relationships.ts: spoofable by design (any caller with the bearer self-stamps the principal — fine for conformance runners deterministically targeting each branch), with a forward-looking guard that future production billing stores must not key on this namespace. - Add a multi-account batching test: [passing, rejected, passing] sequence proves the per-account `continue` loop does not poison passing siblings and result ordering matches request ordering — closes the test surface around the load-bearing per-account semantics of sync_accounts. 22 tests pass; full server unit suite (3088 tests) clean except for one pre-existing flaky LLM test in addie-router.test.ts unrelated to this PR. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
bokelley
added a commit
that referenced
this pull request
May 2, 2026
…aining-agent (#3851) Three bugs surfaced by running the storyboard against the running training-agent reference implementation. Lint couldn't catch any of them — exactly the brittleness the validation was meant to surface before shipping cold. 1. check: error_code does not accept a `path` parameter for per-account error extraction. The runner pulls the code from default paths (adcp_error.code / errors[0].code at root). For sync_accounts' per-account errors at accounts[0].errors[0].code, switch to check: field_value with the explicit path. Two occurrences fixed. 2. expect_error: true requires a transport-level error marker (MCP isError / A2A failed). sync_accounts produces transport-level success with per-account errors inside the success envelope — accounts[].errors[] is the per-account-error shape, not transport failure. Removed expect_error from both gate phases with explanatory comments. 3. Idempotency-key reuse across reject/recover phases produced IDEMPOTENCY_CONFLICT. The recover phase submits a different payload (different `billing` value), and per error-handling.mdx, same key + different payload within the replay window MUST reject. Recover phase now uses a fresh idempotency_key with a distinct `#billing_gate_dispatch_passthrough_recover` stability tag. Also corrected three places in the narrative + storyboard structure that previously asserted the wrong "retry under same key" semantics. Plus a runner-side documentation gap: today's storyboard runner does not auto-extract auth.api_key from the test kit. Callers pass it explicitly via --auth. The test-kit comment now spells out the canonical CLI invocation: npx adcp storyboard run <agent-url> \\ --auth demo-billing-passthrough-v1 \\ --file static/compliance/source/universal/billing-gate-dispatch.yaml Test-kit api_key updated to demo-billing-passthrough-v1 to match the training-agent's recognized prefix family. commercial_relationship: "passthrough_only" uncommented as the default-active precondition. Storyboard now passes 3/3 strict assertions against the training-agent per-agent-gate flow (capability_discovery + per_agent_gate_reject + per_agent_gate_recover); capability_gate phase grades not_applicable correctly when the seller advertises all three billing values. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ate testing Mirrors server/src/training-agent/index.ts:84 — any bearer matching the demo-test-kit pattern stamps `static:demo:<token>` as the principal. Lets the standalone runner exercise the per-buyer-agent billing gate locally over real HTTP without spinning up the full server's auth chain. Used to validate #3839 against #3851 end-to-end: npx tsx server/src/training-agent/standalone.ts & curl -X POST http://localhost:4100/mcp \\ -H 'Authorization: Bearer demo-billing-passthrough-v1' \\ -d '{"jsonrpc":"2.0",...,"params":{"name":"sync_accounts",...}}' Confirmed end-to-end: the auth chain → principal → gate path produces BILLING_NOT_PERMITTED_FOR_AGENT with the right error.details shape on the wire (not just in the in-process simulateCallTool harness). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
5 tasks
bokelley
added a commit
that referenced
this pull request
May 2, 2026
…ionResolver naming guidance (#3839) * spec(accounts): billing-gate conformance storyboard + BrandAuthorizationResolver naming guidance Tier-3 follow-up to #3828 / #3831 (BuyerAgentRegistry spec backing). New universal storyboard `billing-gate-dispatch` exercises the two-gate dispatch contract on `sync_accounts.billing` rejection: seller-wide capability gate (`BILLING_NOT_SUPPORTED` with `error.details.scope: "capability"` and supported_billing echo) and per-buyer-agent commercial-relationship gate (`BILLING_NOT_PERMITTED_FOR_AGENT` with the clamped rejected_billing + optional suggested_billing shape). The recover phase chains off the per-agent reject under the same idempotency_key (matching the spec contract that recovery is a retry of the same upsert with a different `billing` value), capturing suggested_billing into context so the seller's nominated fallback is what actually provisions. Negative-shape assertions actively forbid permitted_billing, rate_card, payment_terms, credit_limit, billing_entity, and account_id from leaking through error.details — the per-agent commercial-state oracles that error-details/billing-not-permitted-for-agent.json's additionalProperties: false is meant to close. New test kit `test-kits/billing-gate-runner.yaml` documents the coordination contract: account.unsupported_billing_probe gates the capability phase; commercial_relationship gates the per-agent phases. Both are precondition-gated rather than capability-gated — sellers without the precondition grade not_applicable on those phases. Conformance and compliance-catalog docs updated; doc-parity lint clean. SDK naming guidance added to accounts-and-agents.mdx Buyer-agent identity section: SDKs SHOULD name the brand-operator authorization Protocol BrandAuthorizationResolver (after the file consulted, brand.json/authorized_operators), NOT AdagentsResolver (adagents.json is publisher-side and models a different relationship). Cross-coordination filed as adcp-client-python#346. Reviewed by ad-tech-protocol-expert and nodejs-testing-expert; all MUST FIX findings addressed (idempotency-key reuse, test-kit fields declared, suggested_billing not strict-pinned to operator), plus key SHOULD FIX findings (action/status/recovery assertions, response_schema on error responses, SDK naming softened to SHOULD). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(storyboard): validate billing-gate-dispatch end-to-end against training-agent (#3851) Three bugs surfaced by running the storyboard against the running training-agent reference implementation. Lint couldn't catch any of them — exactly the brittleness the validation was meant to surface before shipping cold. 1. check: error_code does not accept a `path` parameter for per-account error extraction. The runner pulls the code from default paths (adcp_error.code / errors[0].code at root). For sync_accounts' per-account errors at accounts[0].errors[0].code, switch to check: field_value with the explicit path. Two occurrences fixed. 2. expect_error: true requires a transport-level error marker (MCP isError / A2A failed). sync_accounts produces transport-level success with per-account errors inside the success envelope — accounts[].errors[] is the per-account-error shape, not transport failure. Removed expect_error from both gate phases with explanatory comments. 3. Idempotency-key reuse across reject/recover phases produced IDEMPOTENCY_CONFLICT. The recover phase submits a different payload (different `billing` value), and per error-handling.mdx, same key + different payload within the replay window MUST reject. Recover phase now uses a fresh idempotency_key with a distinct `#billing_gate_dispatch_passthrough_recover` stability tag. Also corrected three places in the narrative + storyboard structure that previously asserted the wrong "retry under same key" semantics. Plus a runner-side documentation gap: today's storyboard runner does not auto-extract auth.api_key from the test kit. Callers pass it explicitly via --auth. The test-kit comment now spells out the canonical CLI invocation: npx adcp storyboard run <agent-url> \\ --auth demo-billing-passthrough-v1 \\ --file static/compliance/source/universal/billing-gate-dispatch.yaml Test-kit api_key updated to demo-billing-passthrough-v1 to match the training-agent's recognized prefix family. commercial_relationship: "passthrough_only" uncommented as the default-active precondition. Storyboard now passes 3/3 strict assertions against the training-agent per-agent-gate flow (capability_discovery + per_agent_gate_reject + per_agent_gate_recover); capability_gate phase grades not_applicable correctly when the seller advertises all three billing values. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This was referenced May 2, 2026
Closed
bokelley
added a commit
to adcontextprotocol/adcp-client
that referenced
this pull request
May 2, 2026
…1315) * feat(decisioning): forward ctx to AccountStore.upsert/list (#1310) Adds an optional `ResolveContext` second argument to `AccountStore.upsert` and `AccountStore.list`; framework forwards `authInfo`, `toolName`, and `agent` (when an `agentRegistry` is configured) — same shape already threaded to `accounts.resolve`, `reportUsage`, `getAccountFinancials`. Unblocks adopters implementing principal-keyed gates on `sync_accounts` / `list_accounts` (e.g. `BILLING_NOT_PERMITTED_FOR_AGENT` per-buyer-agent gate from adcontextprotocol/adcp#3851) without re-deriving identity from the request. Backwards-compatible — `ctx` is optional on the platform side, so existing implementations that don't accept the second arg keep working. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * chore: refresh skill-example baseline for AccountStore ctx signature The build-seller-agent SKILL.md example types `upsert` / `list` as `(params, ctx) => …` but returns the wrong shapes (a pre-existing, already-baselined bug in the example). With ctx now declared on the SDK signature, TS infers the example's params as concrete types instead of `any`, so the diagnostic prefix changes and four implicit- any TS7006 entries drop out. Same example bug, sharper error text. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * docs: address expert-review notes on AccountStore ctx forwarding - JSDoc on `upsert?` recommends `ctx.agent` (registry-resolved durable identity) over `ctx.authInfo.credential` (raw transport credential) for commercial-relationship gates. Per ad-tech-protocol-expert review on PR #1315. - `SyncAccountsResultRow` carries an explicit MUST-NOT-LEAK comment for `authInfo`, mirroring the rule already documented on `Account.authInfo`. Per security-reviewer defense-in-depth note on PR #1315. - Changeset adds a security-relevant migration note: pre-release, adopters with no way to scope `list` per-principal returned every account to every authenticated caller. Scoping is now possible but opt-in — multi-tenant adopters should add it in the upgrade. 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
added a commit
to adcontextprotocol/adcp-client
that referenced
this pull request
May 2, 2026
Routes all four AccountStore handler call sites (`syncAccounts`, `listAccounts`, `reportUsage`, `getAccountFinancials`) plus the two dispatcher resolver paths (`resolveAccount`, `resolveAccountFromAuth`) through a single internal `toResolveCtx` helper. Closes the symmetric gap flagged on PR #1315 review: `reportUsage` and `getAccountFinancials` previously received `authInfo` and `toolName` but not `agent`, even though `accounts.resolve` and the new `upsert` / `list` wiring forward all three. Adopters implementing principal-keyed financial-read or usage gates (e.g. on top of the `BILLING_NOT_PERMITTED_FOR_AGENT` machinery from adcontextprotocol/adcp#3851) now get the resolved buyer-agent identity on every account-store surface. Structural improvement, not just additive: any future addition to `ResolveContext` lands on every account-store method automatically. The previous inline literals at six call sites were what produced the asymmetric `agent` gap in the first place. JSDoc on `reportUsage?` and `getAccountFinancials?` updated to mention `ctx.agent` and steer adopters toward it for commercial-relationship gates (consistent with the guidance landed on `upsert?` in PR #1315). Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
bokelley
added a commit
that referenced
this pull request
May 3, 2026
…g gates flow through per-tenant routes (#3984) * feat(training-agent): wire accounts.upsert on all v6 tenants — billing gates flow through /api/training-agent/<tenant>/mcp Tier-3 follow-up to #3851 (BILLING_NOT_SUPPORTED + BILLING_NOT_PERMITTED_FOR_AGENT gates). Closes the gap that previously left sync_accounts exposed only on the deprecated legacy /mcp route. Unblocked by SDK 6.7.0's accounts.upsert(refs, ctx) ctx-forwarding (originally filed at adcp-client#1310, shipped as Phase 1 of #1269). Shared helper (v6-account-helpers.ts) — single syncAccountsUpsert const delegating to the v5 handleSyncAccounts so capability + per-buyer-agent gate semantics are identical on every v6 tenant route as on legacy /mcp. All six v6 tenant platforms wire the helper — sales, signals (in v6-platform.ts), governance, creative, creative-builder, brand. accounts.upsert: syncAccountsUpsert on each platform's AccountStore. Framework auto-registers sync_accounts on every per-tenant route now that upsert is defined. Tenant router auth bridge (tenants/router.ts) — bridges res.locals.trainingPrincipal (set by the conductor's bearer-auth middleware) onto req.auth.{token, clientId, scopes} so the SDK framework surfaces it as ctx.authInfo.clientId to platform handlers. Without this bridge the framework runs without auth context — the conductor's auth middleware runs upstream of the framework's MCP transport which doesn't otherwise see res.locals. Shape mirrors serve.js attachAuthInfo verbatim (token: '', scopes: []) so any future framework shape-check holds against the bridged value. Integration test (sync-accounts-gates.test.ts) — boots the real tenant router, sends real HTTP, asserts the per-agent gate fires on /api/training-agent/sales/mcp with the clamped error.details shape, autonomous recovery via suggested_billing succeeds, agent-billable bearer accepts all three values, and the uniform-response rule for unrecognized principals holds. Plus an it.each smoke over all six v6 tenant routes confirming the gate fires symmetrically. Stays in CI. Reviewed by code-reviewer; SHOULD FIX (multi-tenant test coverage, SSE/JSON parser disambiguation, stale spike comment) and NICE TO HAVE (token: '' on bridge, refs cast doc note) addressed before push. Verified end-to-end: 10/10 integration tests pass, 22/22 unit tests pass (#3851 regression), full server suite clean (3133 passed, 42 skipped, 0 failed). Type check clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(training-agent): add sync_accounts to TOOL_CATALOG for all v6 tenants Auto-registered by the framework on every tenant whose accounts.upsert is wired (now all six per the parent commit). The drift test in training-agent-tool-catalog-drift.test.ts asserts the catalog matches live tools/list per tenant; without this entry the live response includes sync_accounts but the catalog doesn't, drift fails. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test: address review findings on v6 sync_accounts gate suite Three reviewer-driven hardenings on PR #3984's integration test plus one doc note: 1. randomUUID() per call for idempotency_key (was hard-coded). Hard-coded keys let a watch-mode rerun or flaky-retry harness hit the SDK's idempotency cache and replay the cached envelope — a stale cached success would mask a real regression. The cache lookup runs before the gate, so fresh keys are the only way these tests assert what they claim. nodejs-testing-expert called this MUST FIX. 2. Single beforeAll boot instead of per-test boots. Each bootServer() call rebuilds JWKS + 6 tenants + framework dispatch (~1-2s). 11 per-test boots → 1 shared boot. Test isolation is preserved by construction: assertions inspect the response envelope, brand domains differ across tests, idempotency keys are fresh, gate rejections happen before any persisted state is touched. 3. vi.stubEnv for PUBLIC_TEST_AGENT_TOKEN. Module-level env mutation leaks across vitest workers and makes test ordering load-bearing for downstream files. vi.stubEnv scopes the change to the suite and afterAll restores it. 4. Add unauthenticated-request 401 test. Regression-pin: confirms requireAuth runs before tenantMcpHandler, the bridge's `principal && !req.auth` condition never fires on unauthed traffic. security-reviewer flagged the gap. 5. v6-account-helpers.ts comment documents that account state is tenant-independent by design (one buyer-seller account, all surfaces) so a future contributor doesn't try to "fix" cross-tenant visibility. 11/11 pass; suite runs in ~1.7s (was ~3s with per-test boots). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This was referenced May 3, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Reference implementation of the two billing gates from spec PR #3831 (BuyerAgentRegistry spec backing for adcp-client #1269). Lands on the legacy
/mcproute viahandleSyncAccounts; v6 per-tenant routes have noaccounts.upsertyet, so wiring there follows in a separate PR.This unblocks draft PR #3839 — the conformance storyboard waits on a reference implementation that proves the spec contract works end-to-end.
What it does
Capability gate (
BILLING_NOT_SUPPORTED). Validatesinput.billingagainstSUPPORTED_BILLINGSand emits the canonical error pererror-details/billing-not-supported.json—error.details.scope: "capability"plus an echoedsupported_billinglist. Also fixes a stale advertisement: the legacy/mcproute was advertisingsupported_billing: ['agent']while the handler silently accepted all three; both surfaces now agree on['agent', 'operator', 'advertiser'].Per-buyer-agent gate (
BILLING_NOT_PERMITTED_FOR_AGENT). Newcommercial-relationships.tsmodule maps the authenticatedprincipalto a commercial-relationship enum:demo-billing-passthrough-*→passthrough_only(no payments relationship; onlyoperatorpermitted)demo-billing-agent-billable-*→agent_billable(any supported value permitted)undefined(no per-agent gate fires)Both prefixes match the existing
DEMO_TEST_KIT_KEY_PATTERNauthenticator atindex.ts:68, so no auth changes needed. The gate emits the clampederror.detailsshape pererror-details/billing-not-permitted-for-agent.json(additionalProperties: false—rejected_billing+ optionalsuggested_billingonly). StricttoEqualassertion in the tests enforces the clamp at the wire level.Uniform-response rule. Principals without an onboarded record return
undefinedand fall through to the seller-wide capability gate only — preventing the per-agent code from acting as an onboarding oracle for unrecognized callers. This is the spec's bright line onBILLING_NOT_PERMITTED_FOR_AGENT.Tests
20 unit tests pass (14 existing + 6 new):
error.details.agentrejected withBILLING_NOT_PERMITTED_FOR_AGENT, fullerror.detailsshape clamp asserted viatoEqual.advertiserrejected with the same shape.operatorsucceeds (autonomous recovery viasuggested_billing).billingvalues succeed.agentsucceeds (uniform-response rule).dry_run: truedoes not suppress billing-gate rejections (locks the gate ordering pre-persistence).Reviewed by
code-reviewer. SHOULD FIX (tenantIdreserved ongetCommercialRelationshipsignature for v6 follow-up) and both NICE TO HAVE items (toEqualclamp,dry_runtest) addressed before opening.Follow-ups (separate PRs)
accounts.upserton v6 platforms so the gates flow through/api/training-agent/<tenant>/mcptoo.tenantIdparameter is reserved ongetCommercialRelationshipfor that PR.commercial_relationship: passthrough_onlyagainst the training-agent endpoint — unblocks #3839.supportedBillingsdeclarations across tenants so a runner can compare wiresupported_billingto handler-enforced gate consistently.Test plan
npx vitest run server/tests/unit/account-handlers.test.ts— 20/20 pass.npx tsc --project server/tsconfig.json --noEmit— 0 errors (afternpm installto pull SDK 6.0.0; lockfile drift unrelated to this PR was reverted to keep the diff focused).🤖 Generated with Claude Code