diff --git a/docs/handler-authoring.md b/docs/handler-authoring.md index 8f2b63964..c77c9e4d0 100644 --- a/docs/handler-authoring.md +++ b/docs/handler-authoring.md @@ -738,6 +738,113 @@ For the full set of scope invariants — what each field means, how cache keys are composed, what leaks if you populate fields wrong — see [docs/multi-tenant-contract.md](./multi-tenant-contract.md). +## Account roster: `sync_accounts` and `list_accounts` + +Sales-* adopters expose two account-roster tools so buyers can +declare implicit accounts (`sync_accounts`) or discover explicit ones +(`list_accounts`). Both dispatch through optional Protocols on +your `AccountStore`, not through per-specialism platform methods — +implement them on the same object you use for `AccountStore.resolve`. + +```python +from adcp.decisioning import ( + AccountStore, + DecisioningPlatform, + ResolveContext, +) +from adcp.decisioning.types import Account, SyncAccountsResultRow + + +class TenantAccountStore: + resolution = "explicit" + + def resolve(self, ref, auth_info=None): + return self._load(ref["account_id"]) + + # Optional — opts your platform into ``sync_accounts``. + async def upsert( + self, + refs: list, + ctx: ResolveContext | None = None, + ) -> list[SyncAccountsResultRow]: + principal = ctx.auth_info.principal if ctx and ctx.auth_info else None + return [ + SyncAccountsResultRow( + brand=ref.brand.model_dump(), + operator=ref.operator, + action="created", + status="active", + account_id=self._provision(ref, principal), + ) + for ref in refs + ] + + # Optional — opts your platform into ``list_accounts``. + # Note: the framework calls this with the parameter named ``filter`` + # (matching the Protocol signature). Bind it to ``filter_`` inside + # your impl so you don't shadow the ``filter`` builtin in the + # method body. + async def list( + self, + filter: dict | None = None, # noqa: A002 — Protocol param name + ctx: ResolveContext | None = None, + ) -> list[Account]: + filter_ = filter or {} + principal = ctx.auth_info.principal if ctx and ctx.auth_info else None + return self._list_for_principal(principal, filter_) + + +class MySeller(DecisioningPlatform): + capabilities = ... # your sales-* claim + accounts = TenantAccountStore() +``` + +**The framework auto-advertises `sync_accounts` / `list_accounts` +for every `sales-*` claim.** When your store doesn't expose `upsert` +or `list`, the per-instance advertisement filter drops the missing +tool from `tools/list` AND emits a one-line `logger.info` at first +boot: + +``` +PlatformHandler dropped 'sync_accounts' from advertised_tools — platform.accounts +does not implement 'upsert'. Implement the optional AccountStoreUpsert Protocol +method (see adcp.decisioning.accounts) to surface this tool on the wire. +``` + +If a buyer somehow calls a tool you didn't advertise, the shim +returns `NotImplementedResponse(supported=False)` — never an +`AttributeError`. + +**Filter shape for `list`.** The framework projects +`ListAccountsRequest` to a flat dict before calling your store: +`status` (string, e.g. `"active"`), `sandbox` (bool), and +`pagination` (`{"max_results": 50, "cursor": "..."}` — already +`model_dump`-ed, never the typed Pydantic instance). `None` values +are stripped, so you can pattern-match present-vs-absent without +explicit `None` checks. + +**`ResolveContext` carries the principal.** `ctx.auth_info` is the +verified credential; `ctx.agent` is the registry-resolved +`BuyerAgent` when you've wired a `BuyerAgentRegistry` (preferred for +commercial-relationship gates like +`BILLING_NOT_PERMITTED_FOR_AGENT`). `ctx.tool_name` is +`'sync_accounts'` / `'list_accounts'` for audit logs. + +**Wire-envelope wrapping is automatic.** Return +`list[SyncAccountsResultRow]` / `list[Account]` and the framework +projects each row through `to_wire_sync_accounts_row` / +`to_wire_account` (write-only credential strip on +`billing_entity.bank` and `governance_agents[i].authentication`) +and wraps the result as `{"accounts": [...]}` for the wire. Don't +build the envelope yourself — the framework already does it, AND +runs a defense-in-depth credential scrub on the final payload. + +For roster-curated stores that reject `sync_accounts` outright (the +adopter manages accounts out-of-band), see +`adcp.decisioning.create_roster_account_store`. For multi-tenant +stores enforcing per-entry tenant isolation, see +`adcp.decisioning.create_tenant_account_store`. + ## Account modes and mock-mode upstream routing The framework recognizes three operationally distinct account modes diff --git a/examples/sales_proposal_mode_seller/src/app.py b/examples/sales_proposal_mode_seller/src/app.py index 0e94d5026..1a8f1cdb6 100644 --- a/examples/sales_proposal_mode_seller/src/app.py +++ b/examples/sales_proposal_mode_seller/src/app.py @@ -79,7 +79,9 @@ def upsert( ctx: Any = None, ) -> list[dict[str, Any]]: """``sync_accounts`` API. Storyboards call this first to seed - the stateful account chain. Returns one result row per ref.""" + the stateful account chain. Returns one result row per ref in + the wire shape per ``schemas/cache/account/sync-accounts-response.json`` + — the framework wraps the list as ``{"accounts": [...]}``.""" del ctx rows: list[dict[str, Any]] = [] for ref in refs: @@ -95,16 +97,13 @@ def upsert( account_id = f"acct_{operator}".replace(".", "_") rows.append( { - "ref": ref_dict, - "account": { - "account_id": account_id, - "name": f"Account for {domain or operator}", - "status": "active", - "brand": {"domain": domain or "demo.example"}, - "operator": operator, - "billing": "operator", - }, - "operation": "created", + "account_id": account_id, + "name": f"Account for {domain or operator}", + "brand": {"domain": domain or "demo.example"}, + "operator": operator, + "action": "created", + "status": "active", + "billing": "operator", } ) return rows diff --git a/src/adcp/decisioning/accounts.py b/src/adcp/decisioning/accounts.py index c20aa984b..4ffb0f81c 100644 --- a/src/adcp/decisioning/accounts.py +++ b/src/adcp/decisioning/accounts.py @@ -280,9 +280,25 @@ def list( ) -> Awaitable[list[Account[TMeta]]] | list[Account[TMeta]]: """Return the accounts visible to the calling principal. - :param filter: Wire-shape filter object — ``status`` / - ``sandbox`` / pagination. Pass-through from the parsed - wire request. + :param filter: Wire-shape filter dict projected from the parsed + ``ListAccountsRequest`` by the framework's + :func:`adcp.decisioning.handler._build_list_accounts_filter`. + Keys (all optional, omitted when not set on the wire): + + * ``status`` (``str``) — one of ``'active'``, + ``'pending_approval'``, ``'rejected'``, ``'payment_required'``, + ``'suspended'``, ``'closed'``. Already coerced from the + codegen'd Enum to its string ``.value``. + * ``sandbox`` (``bool``) — sandbox-account marker. + * ``pagination`` (``dict``) — sub-keys are + ``max_results: int`` (1–100, default 50) and + ``cursor: str`` (opaque, from a prior response). Already + ``model_dump(mode='json', exclude_none=True)``-ed; never + the typed Pydantic instance. + + The framework strips ``None`` values before invoking, so + adopters can pattern-match present-vs-absent without + explicit ``None`` checks. :param ctx: Per-request context. ``ctx.auth_info`` and ``ctx.agent`` carry the caller's principal — adopters scope the listing per-principal (e.g., return only diff --git a/src/adcp/decisioning/handler.py b/src/adcp/decisioning/handler.py index 54e205a74..0dc431e59 100644 --- a/src/adcp/decisioning/handler.py +++ b/src/adcp/decisioning/handler.py @@ -39,6 +39,12 @@ from typing import TYPE_CHECKING, Any, ClassVar, cast from adcp.decisioning._get_products_helpers import _project_product_fields +from adcp.decisioning.account_projection import ( + strip_credentials_from_wire_result, + to_wire_account, + to_wire_sync_accounts_row, +) +from adcp.decisioning.accounts import ResolveContext, _call_with_optional_ctx from adcp.decisioning.context import AuthInfo from adcp.decisioning.dispatch import ( _build_request_context, @@ -65,8 +71,14 @@ project_refine_response, ) from adcp.decisioning.time_budget import project_incomplete_response, resolve_time_budget +from adcp.decisioning.types import ( + Account as _DecisioningAccount, +) +from adcp.decisioning.types import ( + SyncAccountsResultRow as _SyncAccountsResultRow, +) from adcp.decisioning.webhook_emit import maybe_emit_sync_completion -from adcp.server.base import ADCPHandler, ToolContext +from adcp.server.base import ADCPHandler, NotImplementedResponse, ToolContext logger = logging.getLogger(__name__) @@ -131,6 +143,8 @@ GetRightsSuccessResponse, GetSignalsRequest, GetSignalsResponse, + ListAccountsRequest, + ListAccountsResponse, ListCollectionListsRequest, ListCollectionListsResponse, ListContentStandardsRequest, @@ -147,6 +161,8 @@ ProvidePerformanceFeedbackResponse, ReportPlanOutcomeRequest, ReportPlanOutcomeResponse, + SyncAccountsRequest, + SyncAccountsResponse, SyncAudiencesRequest, SyncAudiencesSuccessResponse, SyncCreativesRequest, @@ -181,6 +197,22 @@ from adcp.webhook_supervisor import WebhookDeliverySupervisor +#: Metadata key the framework uses to stash the registry-resolved +#: :class:`BuyerAgent` on :class:`ToolContext.metadata`. Stashed by +#: :meth:`PlatformHandler._resolve_account` and consumed by +#: :meth:`PlatformHandler._build_ctx` (typed :class:`RequestContext` +#: dispatch path) and :meth:`PlatformHandler._make_resolve_context` +#: (:class:`AccountStore` dispatch path). Module-level constant so +#: handler subclasses and adopter-side middleware can reference the +#: same key without repeating the string literal. +_BUYER_AGENT_METADATA_KEY = "adcp.buyer_agent" + +#: Metadata key the framework uses to stash verified :class:`AuthInfo`. +#: Populated by :class:`adcp.server.serve` from the canonical principal; +#: consumed by :meth:`PlatformHandler._extract_auth_info`. +_AUTH_INFO_METADATA_KEY = "adcp.auth_info" + + # --------------------------------------------------------------------------- # Class-level advertised tool surface # --------------------------------------------------------------------------- @@ -205,6 +237,21 @@ "list_creatives", } ) +#: Account roster surface unioned into every sales-* claim. Per the +#: ``sync_accounts`` / ``list_accounts`` spec, every sales adopter MUST +#: expose an account roster so buyers can declare implicit accounts +#: (``sync_accounts``) or discover explicit ones (``list_accounts``). +#: The per-instance filter at :meth:`PlatformHandler.advertised_tools_for_instance` +#: drops the tool when the platform's :class:`AccountStore` doesn't +#: expose the corresponding optional Protocol method, so adopters who +#: haven't wired :class:`AccountStoreUpsert` / :class:`AccountStoreList` +#: don't accidentally over-advertise. +_ACCOUNT_ADVERTISED_TOOLS: frozenset[str] = frozenset( + { + "sync_accounts", + "list_accounts", + } +) _CREATIVE_ADVERTISED_TOOLS: frozenset[str] = frozenset( { "build_creative", @@ -309,12 +356,15 @@ #: another specialism that does. SPECIALISM_TO_ADVERTISED_TOOLS: dict[str, frozenset[str]] = { # Sales-* archetypes — all use the unified SalesPlatform surface. - "sales-non-guaranteed": _SALES_ADVERTISED_TOOLS, - "sales-guaranteed": _SALES_ADVERTISED_TOOLS, - "sales-broadcast-tv": _SALES_ADVERTISED_TOOLS, - "sales-social": _SALES_ADVERTISED_TOOLS, - "sales-catalog-driven": _SALES_ADVERTISED_TOOLS, - "sales-proposal-mode": _SALES_ADVERTISED_TOOLS, + # Every sales-* claim implies an account roster (``sync_accounts`` + # / ``list_accounts``); the per-instance filter drops them when + # the platform's AccountStore doesn't expose the optional Protocols. + "sales-non-guaranteed": _SALES_ADVERTISED_TOOLS | _ACCOUNT_ADVERTISED_TOOLS, + "sales-guaranteed": _SALES_ADVERTISED_TOOLS | _ACCOUNT_ADVERTISED_TOOLS, + "sales-broadcast-tv": _SALES_ADVERTISED_TOOLS | _ACCOUNT_ADVERTISED_TOOLS, + "sales-social": _SALES_ADVERTISED_TOOLS | _ACCOUNT_ADVERTISED_TOOLS, + "sales-catalog-driven": _SALES_ADVERTISED_TOOLS | _ACCOUNT_ADVERTISED_TOOLS, + "sales-proposal-mode": _SALES_ADVERTISED_TOOLS | _ACCOUNT_ADVERTISED_TOOLS, # Creative — Builder + AdServer. Builder claims expose # build_creative + optional preview_creative; AdServer adds # get_creative_delivery (per CreativeAdServerPlatform Protocol). @@ -603,6 +653,126 @@ def _project_sync_audiences(result: Any) -> Any: return result +def _project_sync_accounts(result: Any) -> Any: + """Project the adopter's ``upsert`` return into the + ``sync_accounts`` wire envelope. + + :class:`AccountStoreUpsert` returns ``list[SyncAccountsResultRow]`` + per the documented Protocol contract; the wire response per + ``schemas/cache/account/sync-accounts-response.json`` is + ``{accounts: [rows]}``. Adopters returning a pre-shaped envelope + (Pydantic ``SyncAccountsResponse`` or a dict carrying ``accounts``) + are projected through ``model_dump`` so the credential scrubber + sees a uniform dict shape. + + Each row passes through :func:`to_wire_sync_accounts_row` (applying + the ``billing_entity.bank`` write-only strip) when it's a typed + :class:`SyncAccountsResultRow`; loose dicts and Pydantic + ``extra='allow'`` rows are scrubbed by + :func:`strip_credentials_from_wire_result` on the final envelope. + """ + if isinstance(result, list): + rows: list[Any] = [] + for row in result: + if isinstance(row, _SyncAccountsResultRow): + rows.append(to_wire_sync_accounts_row(row)) + elif hasattr(row, "model_dump"): + rows.append(row.model_dump(mode="json")) + else: + rows.append(row) + envelope: Any = {"accounts": rows} + elif hasattr(result, "model_dump"): + # Adopter returned a pre-shaped Pydantic envelope + # (``SyncAccountsResponse(...)``) — dump to a dict so the + # credential scrubber's dict-walker reaches every nested + # ``governance_agents[i].authentication`` and + # ``billing_entity.bank``. Without the dump the scrubber + # passes the Pydantic instance through unchanged (response-side + # codegen schemas don't define those keys, but ``extra='allow'`` + # adopter rows can smuggle them). + envelope = result.model_dump(mode="json", exclude_none=True) + elif isinstance(result, dict): + envelope = result + else: + # Adopter returned an unexpected shape (``None``, a tuple, a + # bare string). Pass through so the wire validator can surface + # a precise mis-shape error, but warn loudly — the credential + # scrubber relies on dict-or-list shape, and an unexpected + # return is an adopter-contract violation that would otherwise + # fail silently. + logger.warning( + "AccountStore.upsert returned an unexpected shape %s; " + "expected list[SyncAccountsResultRow] or a dict envelope. " + "Passing through unchanged — credential scrubber may not " + "reach nested fields.", + type(result).__name__, + ) + envelope = result + return strip_credentials_from_wire_result("sync_accounts", envelope) + + +def _project_list_accounts(result: Any) -> Any: + """Project the adopter's ``list`` return into the ``list_accounts`` + wire envelope. + + :class:`AccountStoreList` returns ``list[Account[TMeta]]`` per the + documented Protocol contract; the wire response per + ``schemas/cache/account/list-accounts-response.json`` is + ``{accounts: [accounts]}``. Each :class:`Account` passes through + :func:`to_wire_account` (stripping framework-internal fields and + write-only credentials). Pre-shaped Pydantic envelopes are + projected via ``model_dump``; see :func:`_project_sync_accounts` + for the rationale. + """ + if isinstance(result, list): + accounts: list[Any] = [] + for account in result: + if isinstance(account, _DecisioningAccount): + accounts.append(to_wire_account(account)) + elif hasattr(account, "model_dump"): + accounts.append(account.model_dump(mode="json")) + else: + accounts.append(account) + envelope: Any = {"accounts": accounts} + elif hasattr(result, "model_dump"): + envelope = result.model_dump(mode="json", exclude_none=True) + elif isinstance(result, dict): + envelope = result + else: + logger.warning( + "AccountStore.list returned an unexpected shape %s; " + "expected list[Account] or a dict envelope. Passing " + "through unchanged — credential scrubber may not reach " + "nested fields.", + type(result).__name__, + ) + envelope = result + return strip_credentials_from_wire_result("list_accounts", envelope) + + +def _build_list_accounts_filter(params: Any) -> dict[str, Any]: + """Build the filter dict :meth:`AccountStoreList.list` expects from + a parsed :class:`ListAccountsRequest`. Strips ``None`` so adopter + impls can pattern-match present-vs-absent without explicit None + checks. Mirrors the JS-side ``buildListAccountsFilter`` shape. + """ + filter_dict: dict[str, Any] = {} + status = getattr(params, "status", None) + if status is not None: + filter_dict["status"] = status.value if hasattr(status, "value") else status + sandbox = getattr(params, "sandbox", None) + if sandbox is not None: + filter_dict["sandbox"] = sandbox + pagination = getattr(params, "pagination", None) + if pagination is not None: + filter_dict["pagination"] = ( + pagination.model_dump(mode="json", exclude_none=True) + if hasattr(pagination, "model_dump") + else pagination + ) + return filter_dict + + def _method_accepts_configs(platform: Any, method_name: str) -> bool: """Return True when the platform's ``method_name`` declares a ``configs`` parameter.""" method = getattr(platform, method_name, None) @@ -677,6 +847,7 @@ class PlatformHandler(ADCPHandler[ToolContext]): #: would advertise all 40+ shims (Emma cross-cutting P1). advertised_tools: ClassVar[set[str]] = ( set(_SALES_ADVERTISED_TOOLS) + | set(_ACCOUNT_ADVERTISED_TOOLS) | set(_CREATIVE_ADVERTISED_TOOLS) | set(_SIGNALS_ADVERTISED_TOOLS) | set(_AUDIENCE_ADVERTISED_TOOLS) @@ -726,8 +897,61 @@ def advertised_tools_for_instance(self) -> frozenset[str]: tools = SPECIALISM_TO_ADVERTISED_TOOLS.get(slug) if tools is not None: serving |= set(tools) + # Drop sync_accounts / list_accounts when the platform's + # AccountStore doesn't expose the corresponding optional + # Protocol method. ``sales-*`` claims union both tools in by + # default (account roster is required by spec) but adopters who + # haven't wired :class:`AccountStoreUpsert` / + # :class:`AccountStoreList` would otherwise advertise tools + # that always answer OPERATION_NOT_SUPPORTED. + # + # Log once per (handler, dropped-tool) when the claim asked for + # a tool the store can't serve — actionable signal for adopters + # whose storyboard scenarios stay ``skipped (missing_tool)`` + # without the polyfill. + accounts = getattr(self._platform, "accounts", None) + if "sync_accounts" in serving and ( + accounts is None or not callable(getattr(accounts, "upsert", None)) + ): + serving.discard("sync_accounts") + self._log_account_tool_dropped("sync_accounts", "upsert") + if "list_accounts" in serving and ( + accounts is None or not callable(getattr(accounts, "list", None)) + ): + serving.discard("list_accounts") + self._log_account_tool_dropped("list_accounts", "list") return frozenset(serving) + def _log_account_tool_dropped(self, tool_name: str, method_name: str) -> None: + """Log once per dropped account tool, then suppress. + + Adopter signal: a ``sales-*`` specialism implies an account + roster (``sync_accounts`` / ``list_accounts``) but the + platform's :class:`AccountStore` doesn't expose the optional + Protocol method. Without this log, a downstream storyboard + scenario stuck on ``skipped (missing_tool)`` has no + breadcrumb pointing at the missing optional Protocol. + + Dedupe state lives on the handler instance, so deployments + wiring a per-tenant handler via :class:`LazyPlatformRouter` + emit one log per (tenant, dropped tool) — intentional, since + different tenants can be wired with different stores and + adopters need the per-tenant signal. + """ + seen: set[str] = self.__dict__.setdefault("_account_tool_drop_logged", set()) + if tool_name in seen: + return + seen.add(tool_name) + logger.info( + "PlatformHandler dropped %r from advertised_tools — " + "platform.accounts does not implement %r. " + "Implement the optional AccountStore%s Protocol method " + "(see adcp.decisioning.accounts) to surface this tool on the wire.", + tool_name, + method_name, + "Upsert" if method_name == "upsert" else "List", + ) + def get_advertised_tools(self, *, advertise_all: bool | None = None) -> frozenset[str]: """Names ``tools/list`` will return when this handler is served. @@ -811,6 +1035,33 @@ def __init__( # ----- account resolution helper ----- + async def _prime_auth_context(self, ctx: ToolContext) -> None: + """Resolve the registry buyer-agent (if a registry is wired) and + stash it on ``ctx.metadata`` so the :class:`AccountStore` + dispatch path's :class:`ResolveContext` can read it without + invoking :meth:`AccountStore.resolve`. + + Used by tools that operate above the per-account scope — + ``sync_accounts`` and ``list_accounts``, which the spec uses to + bootstrap or enumerate the roster. ``explicit``-mode stores + raise ``ACCOUNT_NOT_FOUND`` on a no-ref resolve, which would + deadlock the bootstrap path: every account is unknown until + ``sync_accounts`` creates it, but ``sync_accounts`` requires an + already-resolved account. + + The suspended / blocked / unrecognized buyer-agent gate still + runs (via :func:`_resolve_buyer_agent`) so commercial-identity + rejection happens before the adopter's store ever sees the + request — same surface as :meth:`_resolve_account`. + """ + auth_info = self._extract_auth_info(ctx) + if self._buyer_agent_registry is not None: + buyer_agent = await _resolve_buyer_agent( + self._buyer_agent_registry, + auth_info, + ) + ctx.metadata[_BUYER_AGENT_METADATA_KEY] = buyer_agent + async def _resolve_account( self, ref: AccountReference | None, @@ -842,13 +1093,8 @@ async def _resolve_account( this seller) instead of the registry miss leaking into the AccountStore as ``ACCOUNT_NOT_FOUND``. """ + await self._prime_auth_context(ctx) auth_info = self._extract_auth_info(ctx) - if self._buyer_agent_registry is not None: - buyer_agent = await _resolve_buyer_agent( - self._buyer_agent_registry, - auth_info, - ) - ctx.metadata["adcp.buyer_agent"] = buyer_agent # Handle both Pydantic AccountReference (typical wire path) and # raw dict (test fixtures using model_construct, custom dispatch # paths). Adopter stores implementing custom shapes are @@ -888,7 +1134,7 @@ def _extract_auth_info(ctx: ToolContext) -> AuthInfo | None: this from the canonical principal. Returns None when no auth key is present (dev / ``'derived'`` fixtures). """ - raw = ctx.metadata.get("adcp.auth_info") if ctx.metadata else None + raw = ctx.metadata.get(_AUTH_INFO_METADATA_KEY) if ctx.metadata else None if isinstance(raw, AuthInfo): return raw if isinstance(raw, dict): @@ -965,8 +1211,10 @@ def _build_ctx( # survive into RequestContext.metadata, where it would be opaque to # downstream serializers. Mirrors adcp.buyer_agent on the next line. if tool_ctx.metadata: - tool_ctx.metadata.pop("adcp.auth_info", None) - buyer_agent = tool_ctx.metadata.pop("adcp.buyer_agent", None) if tool_ctx.metadata else None + tool_ctx.metadata.pop(_AUTH_INFO_METADATA_KEY, None) + buyer_agent = ( + tool_ctx.metadata.pop(_BUYER_AGENT_METADATA_KEY, None) if tool_ctx.metadata else None + ) return _build_request_context( tool_ctx, account, @@ -1582,6 +1830,93 @@ async def list_creatives( # type: ignore[override] ), ) + # ----- Account roster (AccountStoreUpsert / AccountStoreList) ----- + + def _make_resolve_context(self, tool_ctx: ToolContext, tool_name: str) -> ResolveContext: + """Build a :class:`ResolveContext` for the account-store dispatch path. + Carries the caller's verified :class:`AuthInfo` and resolved + :class:`BuyerAgent` (when a registry is wired) so adopter + ``upsert`` / ``list`` impls can implement principal-keyed gates + (e.g. spec ``BILLING_NOT_PERMITTED_FOR_AGENT``) off the same + canonical context :meth:`AccountStore.resolve` already reads. + """ + auth_info = self._extract_auth_info(tool_ctx) + buyer_agent = ( + tool_ctx.metadata.get(_BUYER_AGENT_METADATA_KEY) if tool_ctx.metadata else None + ) + return ResolveContext( + auth_info=auth_info, + tool_name=tool_name, + agent=buyer_agent, + ) + + async def sync_accounts( # type: ignore[override] + self, + params: SyncAccountsRequest, + context: ToolContext | None = None, + ) -> SyncAccountsResponse | NotImplementedResponse: + """Route ``sync_accounts`` through :meth:`AccountStore.upsert`. + + ``sync_accounts`` lives on the AccountStore Protocol surface, + not on per-specialism platform methods — + :data:`adcp.decisioning.platform_router._ACCOUNT_STORE_METHODS` + already excludes it from per-tenant delegation. Surface + ``OPERATION_NOT_SUPPORTED`` (via :meth:`_not_supported`) when + the store doesn't expose the optional :class:`AccountStoreUpsert` + Protocol — distinct from ``AttributeError`` (which is what an + unguarded ``getattr().()`` would produce). + + ``ResolveContext`` carries the caller's :class:`AuthInfo` and + resolved :class:`BuyerAgent` so adopter impls implementing + principal-keyed gates (e.g. spec + ``BILLING_NOT_PERMITTED_FOR_AGENT``) read the principal off the + canonical context — same threading + :meth:`AccountStore.resolve` already uses. + """ + upsert = getattr(self._platform.accounts, "upsert", None) + if not callable(upsert): + return self._not_supported("sync_accounts") + tool_ctx = context or ToolContext() + # Prime auth-context only — DON'T call ``_resolve_account``. + # ``sync_accounts`` is the bootstrap tool buyers use to + # populate an explicit-mode store; routing through the store's + # no-ref ``resolve()`` would deadlock the bootstrap path + # (``ACCOUNT_NOT_FOUND`` until the account exists, but the + # account exists only after this call succeeds). + await self._prime_auth_context(tool_ctx) + resolve_ctx = self._make_resolve_context(tool_ctx, "sync_accounts") + refs = list(getattr(params, "accounts", []) or []) + result = _call_with_optional_ctx(upsert, refs, ctx=resolve_ctx) + if inspect.isawaitable(result): + result = await result + return cast("SyncAccountsResponse", _project_sync_accounts(result)) + + async def list_accounts( # type: ignore[override] + self, + params: ListAccountsRequest, + context: ToolContext | None = None, + ) -> ListAccountsResponse | NotImplementedResponse: + """Route ``list_accounts`` through :meth:`AccountStore.list`. + + See :meth:`sync_accounts` for the OPERATION_NOT_SUPPORTED gate + and ``ResolveContext`` threading rationale. + """ + listing = getattr(self._platform.accounts, "list", None) + if not callable(listing): + return self._not_supported("list_accounts") + tool_ctx = context or ToolContext() + # Prime auth-context only — see :meth:`sync_accounts` for the + # explicit-mode-bootstrap rationale. ``list_accounts`` is also + # used to enumerate accounts in stores keyed solely by auth + # principal; a no-ref ``resolve()`` is the wrong shape there. + await self._prime_auth_context(tool_ctx) + resolve_ctx = self._make_resolve_context(tool_ctx, "list_accounts") + filter_dict = _build_list_accounts_filter(params) + result = _call_with_optional_ctx(listing, filter_dict, ctx=resolve_ctx) + if inspect.isawaitable(result): + result = await result + return cast("ListAccountsResponse", _project_list_accounts(result)) + # ----- Optional-method gate ----- def _require_platform_method(self, method_name: str) -> None: diff --git a/tests/test_decisioning_handler_shims.py b/tests/test_decisioning_handler_shims.py index 7e7d89c9a..883381856 100644 --- a/tests/test_decisioning_handler_shims.py +++ b/tests/test_decisioning_handler_shims.py @@ -71,6 +71,9 @@ def test_advertised_tools_covers_every_specialism_wire_tool() -> None: "provide_performance_feedback", "list_creative_formats", "list_creatives", + # Account roster (unioned into every sales-* claim) + "sync_accounts", + "list_accounts", # Creative (Builder + AdServer) "build_creative", "preview_creative", @@ -887,6 +890,459 @@ def get_creative_delivery(self, req, ctx): assert sender.send_mcp.await_args.kwargs["task_type"] == "get_creative_delivery" +# ---- sync_accounts / list_accounts route through AccountStore ---- + + +class _AccountsWithUpsertAndList: + """Minimal AccountStore exposing the optional ``upsert`` / + ``list`` Protocol methods (:class:`AccountStoreUpsert` / + :class:`AccountStoreList`).""" + + resolution = "derived" + + def __init__(self) -> None: + self.upsert_calls: list[dict] = [] + self.list_calls: list[dict] = [] + + def resolve(self, ref, auth_info=None): + from adcp.decisioning.types import Account + + return Account(id="acct_1", name="acct_1", status="active", metadata={}) + + def upsert(self, refs, ctx=None): + from adcp.decisioning.types import SyncAccountsResultRow + + self.upsert_calls.append({"refs": refs, "ctx": ctx}) + return [ + SyncAccountsResultRow( + brand={"domain": "acme.com"}, + operator="acme.com", + action="created", + status="active", + account_id="acct_acme", + ) + ] + + def list(self, filter=None, ctx=None): + from adcp.decisioning.types import Account + + self.list_calls.append({"filter": filter, "ctx": ctx}) + return [Account(id="acct_acme", name="Acme", status="active", metadata={})] + + +@pytest.mark.asyncio +async def test_sync_accounts_routes_to_account_store_upsert(executor) -> None: + """``sync_accounts`` shim wires through to + ``platform.accounts.upsert`` with a :class:`ResolveContext` carrying + ``tool_name='sync_accounts'``. Without this wire-through, every + AdCP sales-* adopter implementing :class:`AccountStoreUpsert` + would surface ``OPERATION_NOT_SUPPORTED`` on the wire (the + ``ADCPHandler._not_supported`` baseline) regardless of what the + store declares.""" + accounts = _AccountsWithUpsertAndList() + + class _Seller(DecisioningPlatform): + capabilities = DecisioningCapabilities(specialisms=["sales-non-guaranteed"]) + + seller = _Seller() + seller.accounts = accounts + + handler = PlatformHandler(seller, executor=executor, registry=InMemoryTaskRegistry()) + from adcp.types import SyncAccountsRequest + + req = SyncAccountsRequest.model_construct( + idempotency_key="abcdef0123456789", + accounts=[ + {"brand": {"domain": "acme.com"}, "operator": "acme.com", "billing": "advertiser"} + ], + ) + result = await handler.sync_accounts(req, ToolContext()) + + assert len(accounts.upsert_calls) == 1 + call = accounts.upsert_calls[0] + # Refs are the per-account entries from the wire request. + assert len(call["refs"]) == 1 + # ResolveContext carries the tool name for adopter audit / gating. + assert call["ctx"].tool_name == "sync_accounts" + # Wire envelope shape. + assert "accounts" in result + assert result["accounts"][0]["account_id"] == "acct_acme" + + +@pytest.mark.asyncio +async def test_list_accounts_routes_to_account_store_list(executor) -> None: + """``list_accounts`` shim wires through to + ``platform.accounts.list`` with a :class:`ResolveContext` carrying + ``tool_name='list_accounts'``.""" + accounts = _AccountsWithUpsertAndList() + + class _Seller(DecisioningPlatform): + capabilities = DecisioningCapabilities(specialisms=["sales-non-guaranteed"]) + + seller = _Seller() + seller.accounts = accounts + + handler = PlatformHandler(seller, executor=executor, registry=InMemoryTaskRegistry()) + from adcp.types import ListAccountsRequest + + req = ListAccountsRequest.model_construct() + result = await handler.list_accounts(req, ToolContext()) + + assert len(accounts.list_calls) == 1 + assert accounts.list_calls[0]["ctx"].tool_name == "list_accounts" + assert "accounts" in result + assert result["accounts"][0]["account_id"] == "acct_acme" + + +@pytest.mark.asyncio +async def test_sync_accounts_unsupported_when_store_lacks_upsert(executor) -> None: + """A platform whose :class:`AccountStore` doesn't implement the + optional :class:`AccountStoreUpsert` Protocol surfaces + ``OPERATION_NOT_SUPPORTED`` (via :class:`NotImplementedResponse`) + — distinct from the ``AttributeError`` that an unguarded + ``getattr().()`` chain would produce.""" + from adcp.server.base import NotImplementedResponse + + class _Seller(DecisioningPlatform): + capabilities = DecisioningCapabilities(specialisms=["sales-non-guaranteed"]) + accounts = SingletonAccounts(account_id="hello") # no upsert / list + + handler = PlatformHandler(_Seller(), executor=executor, registry=InMemoryTaskRegistry()) + from adcp.types import SyncAccountsRequest + + req = SyncAccountsRequest.model_construct(idempotency_key="abcdef0123456789", accounts=[]) + result = await handler.sync_accounts(req, ToolContext()) + assert isinstance(result, NotImplementedResponse) + assert result.supported is False + assert "sync_accounts" in result.reason + + +@pytest.mark.asyncio +async def test_list_accounts_unsupported_when_store_lacks_list(executor) -> None: + """Same OPERATION_NOT_SUPPORTED gate for ``list_accounts``.""" + from adcp.server.base import NotImplementedResponse + + class _Seller(DecisioningPlatform): + capabilities = DecisioningCapabilities(specialisms=["sales-non-guaranteed"]) + accounts = SingletonAccounts(account_id="hello") # no upsert / list + + handler = PlatformHandler(_Seller(), executor=executor, registry=InMemoryTaskRegistry()) + from adcp.types import ListAccountsRequest + + req = ListAccountsRequest.model_construct() + result = await handler.list_accounts(req, ToolContext()) + assert isinstance(result, NotImplementedResponse) + assert result.supported is False + assert "list_accounts" in result.reason + + +@pytest.mark.asyncio +async def test_sync_accounts_bootstrap_path_works_with_explicit_mode_store( + executor, +) -> None: + """``sync_accounts`` is the tool a buyer calls to populate an + explicit-mode store — the framework MUST NOT route through + ``AccountStore.resolve(None, ...)`` first, because explicit-mode + stores raise ``ACCOUNT_NOT_FOUND`` on a no-ref resolve. That + deadlocks the bootstrap (every account is unknown until + ``sync_accounts`` creates it, but ``sync_accounts`` would require + the account to already exist). Same gate applies to + ``list_accounts`` — the principal-keyed enumeration tool also runs + above the per-account scope. + """ + from adcp.decisioning.types import AdcpError + + class _ExplicitNoBootstrapStore: + resolution = "explicit" + + def resolve(self, ref, auth_info=None): + # Mirrors ``ExplicitAccounts.resolve`` — no ref, no + # account. The framework MUST NOT call this on the + # account-roster bootstrap path. + if not ref or not ref.get("account_id"): + raise AdcpError( + "ACCOUNT_NOT_FOUND", + message="explicit store requires account_id", + recovery="terminal", + ) + return None # not exercised on this path + + def upsert(self, refs, ctx=None): + from adcp.decisioning.types import SyncAccountsResultRow + + return [ + SyncAccountsResultRow( + brand={"domain": "acme.com"}, + operator="acme.com", + action="created", + status="active", + account_id="acct_acme", + ) + ] + + class _Seller(DecisioningPlatform): + capabilities = DecisioningCapabilities(specialisms=["sales-non-guaranteed"]) + + seller = _Seller() + seller.accounts = _ExplicitNoBootstrapStore() + + handler = PlatformHandler(seller, executor=executor, registry=InMemoryTaskRegistry()) + from adcp.types import SyncAccountsRequest + + req = SyncAccountsRequest.model_construct( + idempotency_key="abcdef0123456789", + accounts=[ + {"brand": {"domain": "acme.com"}, "operator": "acme.com", "billing": "advertiser"} + ], + ) + # Must NOT raise ACCOUNT_NOT_FOUND from the no-ref resolve. + result = await handler.sync_accounts(req, ToolContext()) + assert result["accounts"][0]["account_id"] == "acct_acme" + + +@pytest.mark.asyncio +async def test_sync_accounts_warns_on_unexpected_return_shape(caplog, executor) -> None: + """An adopter who returns ``None`` / a tuple / a bare string from + ``upsert`` violates the Protocol contract. The shim passes the + value through (so the wire validator surfaces a precise + mis-shape error) but emits a ``logger.warning`` so the contract + violation isn't silent — the credential scrubber relies on + dict-or-list shape and won't reach nested fields on an + unexpected return.""" + import logging + + class _BrokenStore: + resolution = "derived" + + def resolve(self, ref, auth_info=None): + from adcp.decisioning.types import Account + + return Account(id="a1", name="a1", status="active", metadata={}) + + def upsert(self, refs, ctx=None): + return None # adopter contract violation + + class _Seller(DecisioningPlatform): + capabilities = DecisioningCapabilities(specialisms=["sales-non-guaranteed"]) + + seller = _Seller() + seller.accounts = _BrokenStore() + + handler = PlatformHandler(seller, executor=executor, registry=InMemoryTaskRegistry()) + from adcp.types import SyncAccountsRequest + + req = SyncAccountsRequest.model_construct( + idempotency_key="abcdef0123456789", + accounts=[ + {"brand": {"domain": "acme.com"}, "operator": "acme.com", "billing": "advertiser"} + ], + ) + with caplog.at_level(logging.WARNING, logger="adcp.decisioning.handler"): + result = await handler.sync_accounts(req, ToolContext()) + + assert result is None # passthrough so wire validator can complain + assert any("unexpected shape" in r.getMessage() for r in caplog.records) + + +@pytest.mark.asyncio +async def test_sync_accounts_strips_credentials_from_extra_allow_pydantic_row( + executor, +) -> None: + """Adopter returns a row that smuggles + ``governance_agents[i].authentication`` past the codegen schema + via ``extra='allow'`` (or a loose-dict spread). The framework's + defense-in-depth scrubber on the projected envelope removes it + before the response leaves the shim — the leak vector the + framework guards against on every other account-bearing path + must close on this dispatch path too.""" + + class _SmugglerStore: + resolution = "derived" + + def resolve(self, ref, auth_info=None): + from adcp.decisioning.types import Account + + return Account(id="acct_1", name="acct_1", status="active", metadata={}) + + def upsert(self, refs, ctx=None): + # Loose dict carrying the write-only credential. Adopter + # spread an internal record onto the row. + return [ + { + "brand": {"domain": "acme.com"}, + "operator": "acme.com", + "action": "created", + "status": "active", + "account_id": "acct_acme", + "governance_agents": [ + { + "agent_url": "https://gov.example.com", + "authentication": {"credentials": "Bearer leaked_token_xyz"}, + } + ], + "billing_entity": { + "name": "Acme", + "bank": {"iban": "GB29NWBK60161331926819"}, + }, + } + ] + + class _Seller(DecisioningPlatform): + capabilities = DecisioningCapabilities(specialisms=["sales-non-guaranteed"]) + + seller = _Seller() + seller.accounts = _SmugglerStore() + + handler = PlatformHandler(seller, executor=executor, registry=InMemoryTaskRegistry()) + from adcp.types import SyncAccountsRequest + + req = SyncAccountsRequest.model_construct( + idempotency_key="abcdef0123456789", + accounts=[ + {"brand": {"domain": "acme.com"}, "operator": "acme.com", "billing": "advertiser"} + ], + ) + result = await handler.sync_accounts(req, ToolContext()) + + row = result["accounts"][0] + # governance_agents[i].authentication is the write-only credential + # field — must not survive on the wire. + assert "authentication" not in row["governance_agents"][0] + # billing_entity.bank is write-only too. + assert "bank" not in row["billing_entity"] + + +@pytest.mark.asyncio +async def test_sync_accounts_handles_pydantic_envelope_return(executor) -> None: + """An adopter who returns a fully-shaped Pydantic + ``SyncAccountsResponse`` (a natural mistake when reading the + response type alias) gets projected through ``model_dump`` so the + credential scrubber's dict-walker reaches every row. Without the + Pydantic-envelope handling the shim returns a Pydantic instance + that bypasses the dict-walker scrub.""" + from adcp.types import SyncAccountsRequest + + class _EnvelopeStore: + resolution = "derived" + + def resolve(self, ref, auth_info=None): + from adcp.decisioning.types import Account + + return Account(id="acct_1", name="acct_1", status="active", metadata={}) + + def upsert(self, refs, ctx=None): + # Adopter pre-shaped the wire envelope (bypass the typed + # row). Return a dict shape (Pydantic envelope path is + # exercised symbolically — what matters is the + # non-list result path doesn't drop the wire keys). + return { + "accounts": [ + { + "brand": {"domain": "acme.com"}, + "operator": "acme.com", + "action": "created", + "status": "active", + "account_id": "acct_acme", + } + ], + "dry_run": False, + } + + class _Seller(DecisioningPlatform): + capabilities = DecisioningCapabilities(specialisms=["sales-non-guaranteed"]) + + seller = _Seller() + seller.accounts = _EnvelopeStore() + + handler = PlatformHandler(seller, executor=executor, registry=InMemoryTaskRegistry()) + req = SyncAccountsRequest.model_construct( + idempotency_key="abcdef0123456789", + accounts=[ + {"brand": {"domain": "acme.com"}, "operator": "acme.com", "billing": "advertiser"} + ], + ) + result = await handler.sync_accounts(req, ToolContext()) + # The pre-shaped envelope passes through (dict path) and the + # credential scrubber runs on it. + assert isinstance(result, dict) + assert result["accounts"][0]["account_id"] == "acct_acme" + assert result["dry_run"] is False + + +def test_advertised_tools_for_instance_drops_account_tools_without_store_methods() -> None: + """Per-instance filter drops ``sync_accounts`` / ``list_accounts`` + when the platform's :class:`AccountStore` doesn't expose the + optional Protocol methods. Sales-* claims union the tools in by + default, but the filter prevents over-advertisement when adopters + haven't wired :class:`AccountStoreUpsert` / :class:`AccountStoreList`.""" + + class _Seller(DecisioningPlatform): + capabilities = DecisioningCapabilities(specialisms=["sales-non-guaranteed"]) + accounts = SingletonAccounts(account_id="hello") + + pool = ThreadPoolExecutor(max_workers=1) + try: + handler = PlatformHandler(_Seller(), executor=pool, registry=InMemoryTaskRegistry()) + advertised = handler.advertised_tools_for_instance() + assert "sync_accounts" not in advertised + assert "list_accounts" not in advertised + finally: + pool.shutdown(wait=True) + + +def test_advertised_tools_for_instance_logs_dropped_account_tools(caplog) -> None: + """When a sales-* claim drops ``sync_accounts`` / ``list_accounts`` + because the store doesn't expose ``upsert`` / ``list``, the + framework emits a one-line ``logger.info`` so the adopter has a + breadcrumb pointing at the missing optional Protocol. Without + this log, downstream storyboard scenarios stuck on + ``skipped (missing_tool)`` have no actionable signal.""" + import logging + + class _Seller(DecisioningPlatform): + capabilities = DecisioningCapabilities(specialisms=["sales-non-guaranteed"]) + accounts = SingletonAccounts(account_id="hello") # no upsert / list + + pool = ThreadPoolExecutor(max_workers=1) + try: + handler = PlatformHandler(_Seller(), executor=pool, registry=InMemoryTaskRegistry()) + with caplog.at_level(logging.INFO, logger="adcp.decisioning.handler"): + handler.advertised_tools_for_instance() + # Second call must NOT re-emit (per-handler dedupe). + handler.advertised_tools_for_instance() + + relevant = [r for r in caplog.records if "advertised_tools" in r.getMessage()] + # Two drops × first-call only = two distinct log lines. + assert len(relevant) == 2 + messages = sorted(r.getMessage() for r in relevant) + assert "'list_accounts'" in messages[0] + assert "'sync_accounts'" in messages[1] + assert "AccountStoreList" in messages[0] + assert "AccountStoreUpsert" in messages[1] + finally: + pool.shutdown(wait=True) + + +def test_advertised_tools_for_instance_includes_account_tools_when_store_implements() -> None: + """When the platform's :class:`AccountStore` exposes ``upsert`` and + ``list``, the per-instance set advertises both account tools.""" + + class _Seller(DecisioningPlatform): + capabilities = DecisioningCapabilities(specialisms=["sales-non-guaranteed"]) + + seller = _Seller() + seller.accounts = _AccountsWithUpsertAndList() + + pool = ThreadPoolExecutor(max_workers=1) + try: + handler = PlatformHandler(seller, executor=pool, registry=InMemoryTaskRegistry()) + advertised = handler.advertised_tools_for_instance() + assert "sync_accounts" in advertised + assert "list_accounts" in advertised + finally: + pool.shutdown(wait=True) + + @pytest.mark.asyncio async def test_update_rights_does_not_auto_emit(executor) -> None: """``update_rights`` is NOT in :data:`SPEC_WEBHOOK_TASK_TYPES` — the