From 40504d9deef521bad05af2f89d6e709ae5c2efc7 Mon Sep 17 00:00:00 2001 From: Brian O'Kelley Date: Sun, 1 Nov 2026 18:22:33 +0000 Subject: [PATCH 1/6] feat(accounts): add accounts.change and use for account requests client.accounts.change(account_id, changes, revision=, allow_separate_account=) changes an account through request_account. A money change, to billing, billing_entity or payment_terms, needs the revision it was approved against and is refused before dispatch without it; the SDK never reads it, which would defeat lost-update protection. Other changes read the revision from list_accounts and retry once on CONFLICT when the changed fields did not move concurrently. Outcomes are AccountUpdated, with the new revision and the 3.3.0-beta.3 effective_at for money terms; AccountSeparate, with not_carried_over and the account_ids-scoped subscriptions that will not see the new account; AccountMatched; AccountPending; and AccountRejected, whose requires_new_account names ACCOUNT_CHANGE_REQUIRES_NEW_ACCOUNT. allow_separate_account defaults to false, and change never re-points the registry: client.accounts.use(key, account_id) is the explicit step. Co-Authored-By: Claude Opus 5.5 (1M context) --- docs/accounts-ensure-and-change.md | 27 ++- src/adcp/__init__.py | 12 ++ src/adcp/account_outcomes.py | 81 +++++++- src/adcp/accounts.py | 264 ++++++++++++++++++++++++ tests/fixtures/public_api_snapshot.json | 4 + tests/test_account_change.py | 198 ++++++++++++++++++ 6 files changed, 582 insertions(+), 4 deletions(-) create mode 100644 tests/test_account_change.py diff --git a/docs/accounts-ensure-and-change.md b/docs/accounts-ensure-and-change.md index 580466434..f8f8791d7 100644 --- a/docs/accounts-ensure-and-change.md +++ b/docs/accounts-ensure-and-change.md @@ -24,11 +24,34 @@ Call `await client.accounts.refresh()` from a cron job or an `account_request.ch ## Separate accounts -`client.accounts.change(...)` with `allow_separate_account`, and `client.accounts.use(...)`, arrive in a later beta. A seller opening a separate account never re-points the registry on its own. +```python +outcome = await client.accounts.change( + "acc_nova", {"brand": {"domain": "nova-athletics.example", "countries": ["BE", "DE", "NL"]}}, + allow_separate_account=True) + +match outcome: + case AccountUpdated(account_id=aid, revision=rev): ... # same account, new revision + case AccountSeparate(account_id=new, previous_account_id=old) as separate: + # Nothing moved. separate.not_carried_over and separate.subscriptions_not_covering + # say what the new account lacks. New buys go to it only after: + await client.accounts.use(separate.key, new) + case AccountMatched(account_id=other): ... # an existing account matched + case AccountPending(action=action): ... + case AccountRejected() as rejected if rejected.requires_new_account: ... +``` + +- **Default.** `allow_separate_account` defaults to `False`, as in the spec. A seller that would accept the change only as a separate account rejects it, and `outcome.requires_new_account` is `True` (`ACCOUNT_CHANGE_REQUIRES_NEW_ACCOUNT`). +- **No silent re-point.** `change()` never re-points the registry. After `AccountSeparate` or `AccountMatched`, the natural key keeps naming the old account until you call `client.accounts.use(key, account_id)`. Which account carries new buys stays your explicit choice. +- **Not carried over.** `AccountSeparate.not_carried_over` lists the account-level configurations the new account lacks. `subscriptions_not_covering` lists the `account_ids`-scoped reporting subscriptions that will not see it. +- **Next step.** `next_step(outcome)` is `none` for `AccountUpdated`, `AccountSeparate` and `AccountMatched`. ## Money changes and revision -A change to `billing`, `billing_entity` or `payment_terms` needs the explicit account `revision` it was approved against. Send it with `request_account` until `change()` lands. +A change to `billing`, `billing_entity` or `payment_terms` needs the explicit account `revision` it was approved against: `await client.accounts.change(account_id, {"payment_terms": "net_60"}, revision=3)`. +- **Without `revision`** the call is refused before anything is sent: `AccountRequestError` with `dispatched=False` and code `ACCOUNT_REVISION_REQUIRED`. The SDK never reads the revision for you, because a human approved the change against a specific revision, and reading it would defeat the lost-update protection. +- **`effective_at`.** `AccountUpdated.effective_at` says when the new money terms take effect. Delivery before it is invoiced under the previous terms. +- **Other changes** (`brand` countries, `operator_unit`, `currency`, `timezone`) read the current `revision` from `list_accounts` when you omit it. On `CONFLICT` they retry once, but only when the fields being changed did not change concurrently. +- **Sellers without `account.requests`.** `change()` is refused (`ACCOUNT_REQUESTS_UNSUPPORTED`); change accounts through `sync_accounts` instead. ## Cold start diff --git a/src/adcp/__init__.py b/src/adcp/__init__.py index 10a190bef..3f6ab3ccc 100644 --- a/src/adcp/__init__.py +++ b/src/adcp/__init__.py @@ -117,6 +117,10 @@ def _resolve_version() -> str: ), "adcp.account_outcomes": ( "AccountAmbiguous", + "AccountChangeOutcome", + "AccountMatched", + "AccountSeparate", + "AccountUpdated", "AccountOutcome", "AccountPending", "AccountReady", @@ -1049,6 +1053,10 @@ def get_adcp_version() -> str: "InMemoryAccountStorage", "ListableAccountStorage", "AccountAmbiguous", + "AccountChangeOutcome", + "AccountMatched", + "AccountSeparate", + "AccountUpdated", "AccountOutcome", "AccountPending", "AccountReady", @@ -1843,7 +1851,11 @@ def get_adcp_version() -> str: ) from adcp.account_outcomes import ( AccountAmbiguous, + AccountChangeOutcome, + AccountMatched, AccountOutcome, + AccountSeparate, + AccountUpdated, AccountPending, AccountReady, AccountRejected, diff --git a/src/adcp/account_outcomes.py b/src/adcp/account_outcomes.py index f4f4bf45a..099fb8a44 100644 --- a/src/adcp/account_outcomes.py +++ b/src/adcp/account_outcomes.py @@ -25,6 +25,10 @@ from typing import Any, Literal __all__ = [ + "AccountChangeOutcome", + "AccountMatched", + "AccountSeparate", + "AccountUpdated", "AccountAmbiguous", "AccountOutcome", "AccountPending", @@ -181,11 +185,84 @@ class AccountRejected: request_id: str | None = None kind: Literal["rejected"] = "rejected" + @property + def requires_new_account(self) -> bool: + """``ACCOUNT_CHANGE_REQUIRES_NEW_ACCOUNT``: the seller would accept the change only + as a separate account; resend with ``allow_separate_account=True``.""" + return any( + str(item.get("code")) == "ACCOUNT_CHANGE_REQUIRES_NEW_ACCOUNT" for item in self.errors + ) + AccountOutcome = AccountReady | AccountPending | AccountAmbiguous | AccountRejected -def next_step(outcome: AccountOutcome) -> NextStep: +@dataclass(frozen=True) +class AccountUpdated: + """``client.accounts.change``: the named account changed in place. + + ``account_id`` is unchanged and ``revision`` is the account's new revision. + ``effective_at`` is when changed money terms (``billing``, + ``billing_entity``, ``payment_terms``) take effect: delivery before it is + invoiced under the previous terms (AdCP 3.3.0-beta.3). + """ + + account_id: str + revision: int | None = None + effective_at: str | None = None + account: Mapping[str, Any] | None = None + request_id: str | None = None + via: AccountVia = "request_account" + kind: Literal["updated"] = "updated" + + +@dataclass(frozen=True) +class AccountSeparate: + """``client.accounts.change``: the seller opened a separate account with the change. + + Nothing moved: ``previous_account_id`` is unchanged, and the registry + still maps the natural ``key`` to it until you call + ``client.accounts.use(key, account_id)``. ``not_carried_over`` names the + account-level configurations the new account does not have, and + ``subscriptions_not_covering`` the ``account_ids``-scoped reporting + subscriptions that will not see it. + """ + + account_id: str + previous_account_id: str + key: Mapping[str, Any] | None = None + account: Mapping[str, Any] | None = None + not_carried_over: tuple[str, ...] = () + subscriptions_not_covering: tuple[str, ...] = () + request_id: str | None = None + via: AccountVia = "request_account" + kind: Literal["separate_account"] = "separate_account" + + +@dataclass(frozen=True) +class AccountMatched: + """``client.accounts.change``: the change matched another existing account. + + As with :class:`AccountSeparate`, the registry is not re-pointed until + ``client.accounts.use(key, account_id)``. Read the matched account's money + terms from :attr:`account` before spending. + """ + + account_id: str + previous_account_id: str + key: Mapping[str, Any] | None = None + account: Mapping[str, Any] | None = None + request_id: str | None = None + via: AccountVia = "request_account" + kind: Literal["matched_existing"] = "matched_existing" + + +AccountChangeOutcome = ( + AccountUpdated | AccountSeparate | AccountMatched | AccountPending | AccountRejected +) + + +def next_step(outcome: AccountOutcome | AccountChangeOutcome) -> NextStep: """What to do next, independent of the extensible ``kind`` vocabulary. * ``none``: the account is ready. @@ -195,7 +272,7 @@ def next_step(outcome: AccountOutcome) -> NextStep: * ``pick_candidate``: choose one of ``candidates``. * ``contact_seller``: rejected, or a buyer action without a URL. """ - if isinstance(outcome, AccountReady): + if isinstance(outcome, (AccountReady, AccountUpdated, AccountSeparate, AccountMatched)): return "none" if isinstance(outcome, AccountAmbiguous): return "pick_candidate" diff --git a/src/adcp/accounts.py b/src/adcp/accounts.py index a79b72779..7a73b6ca1 100644 --- a/src/adcp/accounts.py +++ b/src/adcp/accounts.py @@ -16,10 +16,14 @@ from adcp.account_identity import account_key, account_key_payload from adcp.account_outcomes import ( AccountAmbiguous, + AccountChangeOutcome, + AccountMatched, AccountOutcome, AccountPending, AccountReady, AccountRejected, + AccountSeparate, + AccountUpdated, AccountVia, PendingAction, _errors, @@ -263,6 +267,37 @@ def _request_outcome( ) +#: Attributes ``change()`` sends; the money ones need an explicit ``revision``. +_CHANGEABLE = frozenset( + { + "brand", + "operator_unit", + "currency", + "timezone", + "billing", + "billing_entity", + "payment_terms", + } +) +_MONEY_FIELDS = frozenset({"billing", "billing_entity", "payment_terms"}) + + +def _request_account_request(payload: Mapping[str, Any]) -> Any: + from adcp.types import RequestAccountRequest + + return RequestAccountRequest.model_validate(dict(payload)) + + +def _change_refused(code: str, message: str, *, agent_id: str | None) -> AccountRequestError: + """A local refusal: nothing was sent (``dispatched=False``).""" + error = AccountRequestError( + "request_account", [{"code": code, "message": message}], agent_id=agent_id + ) + error.dispatched = False + error.recovery = "correctable" + return error + + def has_natural_reference(request: Any) -> bool: """Return True when ``request`` carries a buyer-declared natural account key.""" if "account" not in getattr(type(request), "model_fields", {}): @@ -443,6 +478,235 @@ async def _known_outcome( request_id=record.request_id, ) + async def change( + self, + account_id: str, + changes: Mapping[str, Any], + *, + revision: int | None = None, + allow_separate_account: bool = False, + ) -> AccountChangeOutcome: + """Change an account's attributes through ``request_account``; returns an outcome. + + ``changes`` holds the attributes to change: ``brand`` (to change + ``countries``; ``domain`` and ``brand_id`` must equal the account's), + ``operator_unit``, ``currency``, ``timezone``, ``billing``, + ``billing_entity`` and ``payment_terms``. + + * **Money changes** (``billing``, ``billing_entity``, + ``payment_terms``) need ``revision``: the account revision the + change was approved against. The SDK never reads it for you, since + that would defeat the lost-update protection; without it the call + is refused before anything is sent. The result's + ``effective_at`` says when the new terms apply. + * **Other changes** read the current ``revision`` from + ``list_accounts`` when you omit it, and retry once on ``CONFLICT`` + if the fields being changed did not change concurrently. + * ``allow_separate_account`` (default ``False``, as in the spec): may + the seller answer by opening a separate account? Without it, such a + seller rejects the change (``outcome.requires_new_account``). + + Outcomes: :class:`~adcp.account_outcomes.AccountUpdated` (same + ``account_id``, new revision), :class:`~adcp.account_outcomes.AccountSeparate` + and :class:`~adcp.account_outcomes.AccountMatched` (a different + account; the registry is not re-pointed until :meth:`use`), + :class:`~adcp.account_outcomes.AccountPending` and + :class:`~adcp.account_outcomes.AccountRejected`. + """ + payload = { + key: _payload(value) if hasattr(value, "model_dump") else value + for key, value in dict(changes).items() + if value is not None + } + unknown = set(payload) - _CHANGEABLE + if not payload or unknown: + raise _change_refused( + "ACCOUNT_CHANGE_INVALID", + ( + f"change() cannot change {sorted(unknown)}. " + if unknown + else "change() needs at least one attribute to change. " + ) + + f"Change one of {sorted(_CHANGEABLE)}.", + agent_id=self._client.agent_config.id, + ) + money = sorted(set(payload) & _MONEY_FIELDS) + if money and revision is None: + raise _change_refused( + "ACCOUNT_REVISION_REQUIRED", + f"Changing {', '.join(money)} needs the account revision the change was approved " + "against, so a concurrent change is never overwritten. Read it from list_accounts " + "and pass revision=.", + agent_id=self._client.agent_config.id, + ) + caps = await self._client.fetch_capabilities() + if not _requests_supported(caps): + raise _change_refused( + "ACCOUNT_REQUESTS_UNSUPPORTED", + "This seller does not advertise account.requests, so accounts change through " + "sync_accounts. Use client.sync_accounts for this seller.", + agent_id=self._client.agent_config.id, + ) + automatic = revision is None + before = await self._read_account(account_id) if automatic else None + if automatic and before is not None: + revision = before.get("revision") + for attempt in (1, 2): + entry: dict[str, Any] = {"account_id": account_id, **payload} + if revision is not None: + entry["revision"] = revision + if allow_separate_account: + entry["allow_separate_account"] = True + result = await self._client.request_account( + _request_account_request( + {"idempotency_key": _ensure_key(self._seller, entry), **entry} + ) + ) + body = _payload(result.data) + request = body.get("request") + if result.success and not body.get("adcp_error") and isinstance(request, Mapping): + return await self._change_outcome(account_id, request) + failure = self._failure("request_account", result, body) + if ( + attempt == 1 + and automatic + and "CONFLICT" in failure.error_codes + and before is not None + ): + current = await self._read_account(account_id) + if current is not None and all( + current.get(name) == before.get(name) for name in payload + ): + # Only the revision moved under us: retry once on the new one. + revision = current.get("revision") + before = current + continue + raise failure + raise AssertionError("unreachable") # pragma: no cover + + async def use(self, key: AccountReference | Mapping[str, Any], account_id: str) -> None: + """Point a natural key at ``account_id``: new buys for ``key`` go to that account. + + The explicit step after :meth:`change` returned + :class:`~adcp.account_outcomes.AccountSeparate` or + :class:`~adcp.account_outcomes.AccountMatched`; the registry never + re-points on its own. + """ + ref = account_key_payload(TypeAdapter(AccountReference).validate_python(key)) + if "account_id" in ref: + raise ValueError("use() maps a buyer-declared natural key, not an account_id") + if not account_id: + raise ValueError("use() needs the account_id new buys go to") + previous = await self.get(ref) + await self._storage.save( + self._seller, + account_key(ref), + AccountRecord( + account_id=account_id, + status="active", + wire_key=None if previous is None else previous.wire_key, + wire_version=None if previous is None else previous.wire_version, + ), + ) + self._touched.add(account_key(ref)) + + async def _read_account(self, account_id: str) -> dict[str, Any] | None: + """The account as ``list_accounts`` shows it to the caller, or ``None``.""" + from adcp.types import ListAccountsRequest + + cursor: str | None = None + for _page in range(100): + payload: dict[str, Any] = {} + if cursor: + payload["pagination"] = {"cursor": cursor} + result = await self._client.list_accounts(ListAccountsRequest.model_validate(payload)) + body = _payload(result.data) + if not result.success or body.get("errors"): + raise self._failure("list_accounts", result, body) + for item in body.get("accounts") or (): + if isinstance(item, Mapping) and item.get("account_id") == account_id: + return dict(item) + pagination = body.get("pagination") or {} + cursor = pagination.get("cursor") if pagination.get("has_more") else None + if not cursor: + return None + return None + + async def _key_for(self, account_id: str) -> dict[str, Any] | None: + for key, record in await self._known(): + if record.account_id == account_id: + return _key_payload(key) + return None + + async def _subscriptions_not_covering(self, previous: str, new: str) -> tuple[str, ...]: + """Subscriptions scoped to ``account_ids`` naming ``previous`` but not ``new``.""" + reporting = getattr(self._client, "reporting", None) + if reporting is None: + return () + try: + readback = await reporting.list_subscriptions() + except Exception: # noqa: BLE001 - informational only + return () + result = [] + for state in readback.subscriptions: + configuration = _payload(getattr(state, "configuration", None) or {}) + ids = (configuration.get("scope") or {}).get("account_ids") + if ids is not None and previous in ids and new not in ids: + result.append(str(state.subscription_id)) + return tuple(sorted(result)) + + async def _change_outcome(self, previous: str, wire: Mapping[str, Any]) -> AccountChangeOutcome: + state = str(wire["state"]) + request_id = str(wire["request_id"]) + if state == "in_review": + return AccountPending( + action=PendingAction.from_wire(wire.get("pending_action") or _SELLER_REVIEW), + via="request_account", + request_id=request_id, + account_id=previous, + ) + if state != "resolved": + return AccountRejected( + errors=_errors(wire.get("errors")) + or ({"code": "ACCOUNT_REQUEST_REJECTED", "message": "The seller declined"},), + via="request_account", + request_id=request_id, + ) + account = wire.get("account") if isinstance(wire.get("account"), Mapping) else None + resolution = str(getattr(wire.get("resolution"), "value", wire.get("resolution"))) + account_id = str(wire.get("account_id") or previous) + if resolution == "updated" or account_id == previous: + effective_at = wire.get("effective_at") + return AccountUpdated( + account_id=account_id, + revision=(account or {}).get("revision"), + effective_at=None if effective_at is None else str(effective_at), + account=account, + request_id=request_id, + ) + key = await self._key_for(previous) + if resolution == "created": + return AccountSeparate( + account_id=account_id, + previous_account_id=previous, + key=key, + account=account, + not_carried_over=tuple( + str(getattr(item, "value", item)) for item in wire.get("not_carried_over") or () + ), + subscriptions_not_covering=await self._subscriptions_not_covering( + previous, account_id + ), + request_id=request_id, + ) + return AccountMatched( + account_id=account_id, + previous_account_id=previous, + key=key, + account=account, + request_id=request_id, + ) + def _failure(self, operation: str, result: Any, body: Mapping[str, Any]) -> AccountRequestError: errors = body.get("errors") or [ result.adcp_error diff --git a/tests/fixtures/public_api_snapshot.json b/tests/fixtures/public_api_snapshot.json index db131ec83..f1c07255f 100644 --- a/tests/fixtures/public_api_snapshot.json +++ b/tests/fixtures/public_api_snapshot.json @@ -37,6 +37,8 @@ "AcceptedLoss": "media_buy.legacy_purchase_continuation_input", "AccountAmbiguous": "adcp.account_outcomes", "AccountAuthorization": "core.account_authorization", + "AccountChangeOutcome": "union[adcp.account_outcomes:AccountMatched,adcp.account_outcomes:AccountPending,adcp.account_outcomes:AccountRejected,adcp.account_outcomes:AccountSeparate,adcp.account_outcomes:AccountUpdated]", + "AccountMatched": "adcp.account_outcomes", "AccountNotFoundError": "adcp.exceptions", "AccountOutcome": "union[adcp.account_outcomes:AccountAmbiguous,adcp.account_outcomes:AccountPending,adcp.account_outcomes:AccountReady,adcp.account_outcomes:AccountRejected]", "AccountPaymentRequiredError": "adcp.exceptions", @@ -51,8 +53,10 @@ "AccountRejected": "adcp.account_outcomes", "AccountRequestError": "adcp.exceptions", "AccountScope": "enums.account_scope", + "AccountSeparate": "adcp.account_outcomes", "AccountSetupRequiredError": "adcp.exceptions", "AccountStorage": "adcp.accounts", + "AccountUpdated": "adcp.account_outcomes", "AccountWithAuthorization": "core.account_with_authorization", "AcquireRightsAcquiredResponse": "brand.acquire_rights_response:AcquireRightsResponse1", "AcquireRightsErrorResponse": "brand.acquire_rights_response:AcquireRightsResponse4", diff --git a/tests/test_account_change.py b/tests/test_account_change.py new file mode 100644 index 000000000..b12fc6d03 --- /dev/null +++ b/tests/test_account_change.py @@ -0,0 +1,198 @@ +"""``client.accounts.change`` and ``use`` (AdCP 3.3.0-beta.3, ``account.requests``). + +A money change needs the revision the human approved; other changes read it. +A separate or matched account never re-points the registry on its own. +""" + +from __future__ import annotations + +from typing import Any +from unittest.mock import AsyncMock + +import pytest + +from adcp import ( + AccountMatched, + AccountPending, + AccountRejected, + AccountRequestError, + AccountSeparate, + AccountUpdated, + next_step, +) +from adcp.exceptions import is_pre_dispatch_error +from adcp.types.core import TaskResult, TaskStatus +from tests.test_account_lifecycle import KEY, make_client +from tests.test_account_outcomes import REQUESTS, _completed, _record + +pytestmark = pytest.mark.asyncio +ACCOUNT = {"account_id": "acc-1", "name": "Acme", "status": "active", "revision": 3} + + +def _client(*responses: Any, accounts: list[dict[str, Any]] | None = None): + client = make_client(requests=REQUESTS) + client.adapter.request_account = AsyncMock(side_effect=list(responses)) + client.adapter.list_accounts = AsyncMock( + return_value=_completed({"accounts": accounts if accounts is not None else [ACCOUNT]}) + ) + client.adapter._parse_response = lambda raw, _type: raw + return client + + +def _resolved(**fields: Any) -> TaskResult: + return _completed({"status": "completed", "request": _record("resolved", **fields)}) + + +async def test_a_money_change_needs_the_approved_revision_and_sends_nothing_without_it() -> None: + client = _client() + with pytest.raises(AccountRequestError) as refused: + await client.accounts.change("acc-1", {"payment_terms": "net_60"}) + assert is_pre_dispatch_error(refused.value) and refused.value.recovery == "correctable" + assert refused.value.error_codes == ["ACCOUNT_REVISION_REQUIRED"] + client.adapter.request_account.assert_not_called() + client.adapter.list_accounts.assert_not_called() + + +async def test_a_money_change_with_revision_is_updated_with_effective_at() -> None: + client = _client( + _resolved( + account_id="acc-1", + resolution="updated", + effective_at="2026-12-01T00:00:00Z", + account={**ACCOUNT, "revision": 4, "payment_terms": "net_60"}, + ) + ) + outcome = await client.accounts.change("acc-1", {"payment_terms": "net_60"}, revision=3) + assert isinstance(outcome, AccountUpdated) + assert (outcome.account_id, outcome.revision, outcome.effective_at) == ( + "acc-1", + 4, + "2026-12-01T00:00:00Z", + ) + assert next_step(outcome) == "none" + sent = client.adapter.request_account.call_args.args[0] + assert sent["revision"] == 3 and sent["payment_terms"] == "net_60" + assert "allow_separate_account" not in sent + client.adapter.list_accounts.assert_not_called() + + +async def test_other_changes_read_the_revision_and_retry_once_on_a_concurrent_bump() -> None: + conflict = TaskResult( + status=TaskStatus.FAILED, + data={"errors": [{"code": "CONFLICT", "message": "stale revision"}]}, + success=False, + ) + client = _client( + conflict, + _resolved(account_id="acc-1", resolution="updated", account={**ACCOUNT, "revision": 5}), + ) + client.adapter.list_accounts = AsyncMock( + side_effect=[ + _completed({"accounts": [ACCOUNT]}), + _completed({"accounts": [{**ACCOUNT, "revision": 4}]}), + ] + ) + outcome = await client.accounts.change("acc-1", {"timezone": "Europe/Paris"}) + assert isinstance(outcome, AccountUpdated) and outcome.revision == 5 + first, second = (call.args[0] for call in client.adapter.request_account.call_args_list) + assert (first["revision"], second["revision"]) == (3, 4) + + +async def test_a_conflict_on_the_changed_field_is_not_retried() -> None: + conflict = TaskResult( + status=TaskStatus.FAILED, + data={"errors": [{"code": "CONFLICT", "message": "stale revision"}]}, + success=False, + ) + client = _client(conflict) + client.adapter.list_accounts = AsyncMock( + side_effect=[ + _completed({"accounts": [{**ACCOUNT, "timezone": "UTC"}]}), + _completed({"accounts": [{**ACCOUNT, "revision": 4, "timezone": "Asia/Tokyo"}]}), + ] + ) + with pytest.raises(AccountRequestError) as error: + await client.accounts.change("acc-1", {"timezone": "Europe/Paris"}) + assert "CONFLICT" in error.value.error_codes + assert client.adapter.request_account.call_count == 1 + + +async def test_a_separate_account_lists_what_is_missing_and_never_repoints() -> None: + client = _client( + _resolved( + account_id="acc-2", + resolution="created", + not_carried_over=["reporting_delivery_configs", "creatives"], + account={"account_id": "acc-2", "name": "Acme BE", "status": "active"}, + ) + ) + await client.accounts.use(KEY, "acc-1") + outcome = await client.accounts.change( + "acc-1", + {"brand": {"domain": "acme.example", "countries": ["BE"]}}, + allow_separate_account=True, + ) + assert isinstance(outcome, AccountSeparate) + assert (outcome.account_id, outcome.previous_account_id) == ("acc-2", "acc-1") + assert outcome.not_carried_over == ("reporting_delivery_configs", "creatives") + assert outcome.key is not None and outcome.key["brand"]["domain"] == "acme.example" + assert client.adapter.request_account.call_args.args[0]["allow_separate_account"] is True + # Nothing moved: the key still names the old account until use(). + assert (await client.accounts.get(KEY)).account_id == "acc-1" + await client.accounts.use(outcome.key, outcome.account_id) + assert (await client.accounts.get(KEY)).account_id == "acc-2" + + +async def test_matched_pending_and_rejected_outcomes() -> None: + client = _client( + _resolved( + account_id="acc-9", resolution="matched", account={**ACCOUNT, "account_id": "acc-9"} + ), + _completed( + { + "status": "completed", + "request": _record( + "in_review", + pending_action={ + "waiting_on": "seller", + "kind": "review", + "message": "Reviewing", + }, + ), + } + ), + _completed( + { + "status": "completed", + "request": _record( + "rejected", + errors=[ + { + "code": "ACCOUNT_CHANGE_REQUIRES_NEW_ACCOUNT", + "message": "Needs a new account", + } + ], + ), + } + ), + ) + matched = await client.accounts.change("acc-1", {"timezone": "UTC"}, revision=3) + assert isinstance(matched, AccountMatched) and matched.account_id == "acc-9" + pending = await client.accounts.change("acc-1", {"timezone": "UTC"}, revision=3) + assert isinstance(pending, AccountPending) and next_step(pending) == "wait_for_seller" + rejected = await client.accounts.change("acc-1", {"timezone": "UTC"}, revision=3) + assert isinstance(rejected, AccountRejected) and rejected.requires_new_account + + +async def test_change_refuses_unknown_fields_and_sellers_without_account_requests() -> None: + client = _client() + with pytest.raises(AccountRequestError) as unknown: + await client.accounts.change("acc-1", {"sandbox": True}) + assert unknown.value.error_codes == ["ACCOUNT_CHANGE_INVALID"] + plain = make_client() + plain.adapter.request_account = AsyncMock() + with pytest.raises(AccountRequestError) as unsupported: + await plain.accounts.change("acc-1", {"timezone": "UTC"}) + assert unsupported.value.error_codes == ["ACCOUNT_REQUESTS_UNSUPPORTED"] + assert is_pre_dispatch_error(unsupported.value) + plain.adapter.request_account.assert_not_called() From d52e84342244280c0b9871f7e5b96907be8db81a Mon Sep 17 00:00:00 2001 From: Brian O'Kelley Date: Sun, 1 Nov 2026 17:08:08 +0000 Subject: [PATCH 2/6] test(reporting): run the row-upload lease test at the wall clock The producer row-upload test stamps its worker lease with the settling harness clock, fixed at 2026-11-01T02:10Z, while the row store checks the lease against the database's clock_timestamp. Once the wall clock passed that instant plus the lease, every run failed with STATE_UNAVAILABLE: no live worker lease covers this row upload, on next and on every pull request. The test now runs its turn at the wall clock, so the lease lies in the real future. Co-Authored-By: Claude Opus 5.5 (1M context) (cherry picked from commit ae15bab6e8a73f974ff6adc30cf761454b089b44) --- .../reporting/test_reporting_row_storage_object_pg.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/tests/conformance/reporting/test_reporting_row_storage_object_pg.py b/tests/conformance/reporting/test_reporting_row_storage_object_pg.py index aa2410045..f4cab4017 100644 --- a/tests/conformance/reporting/test_reporting_row_storage_object_pg.py +++ b/tests/conformance/reporting/test_reporting_row_storage_object_pg.py @@ -3,6 +3,7 @@ from __future__ import annotations import os +from datetime import datetime, timezone from pathlib import Path from typing import Any @@ -442,13 +443,18 @@ async def test_the_producer_stages_object_uploads_outside_its_publication_transa from tests.test_reporting_settling import _capabilities, _harness, _revisions async with isolated_reporting_pool() as pool: - producer, store, _, _ = await _harness( + producer, store, _, clock = await _harness( _capabilities(restatement_window="P3D"), store_factory=lambda clock: PgReportingLedgerStore( pool=pool, clock=clock, row_storage=options(tmp_path) ), ) assert isinstance(producer, ReportingProducer) + # The row upload checks the worker lease against the database's + # clock_timestamp(), so the lease the injected clock stamps must lie in + # the real future: run the turn at the wall clock, not the harness's + # fixed instant, which is already in the past. + clock[0] = datetime.now(timezone.utc) turn = await producer.run_worker() assert turn.revisions_committed (revision,) = await _revisions(store) From 80ce9f02c4dbc9eeee3057af6233299787395dd8 Mon Sep 17 00:00:00 2001 From: Brian O'Kelley Date: Sun, 1 Nov 2026 18:34:57 +0000 Subject: [PATCH 3/6] fix(accounts): keep the account status when use re-points a key use stamped every re-pointed account active, which let the spend gate pass a matched account that was payment_required or pending. It now records the status list_accounts shows, or the one passed in, and leaves an account the caller cannot list without a status. Co-Authored-By: Claude Opus 5.5 (1M context) --- docs/accounts-ensure-and-change.md | 2 +- src/adcp/accounts.py | 18 +++++++++++++++--- tests/test_account_change.py | 11 +++++++++-- 3 files changed, 25 insertions(+), 6 deletions(-) diff --git a/docs/accounts-ensure-and-change.md b/docs/accounts-ensure-and-change.md index f8f8791d7..8ac773a05 100644 --- a/docs/accounts-ensure-and-change.md +++ b/docs/accounts-ensure-and-change.md @@ -41,7 +41,7 @@ match outcome: ``` - **Default.** `allow_separate_account` defaults to `False`, as in the spec. A seller that would accept the change only as a separate account rejects it, and `outcome.requires_new_account` is `True` (`ACCOUNT_CHANGE_REQUIRES_NEW_ACCOUNT`). -- **No silent re-point.** `change()` never re-points the registry. After `AccountSeparate` or `AccountMatched`, the natural key keeps naming the old account until you call `client.accounts.use(key, account_id)`. Which account carries new buys stays your explicit choice. +- **No silent re-point.** `change()` never re-points the registry; `use()` records the account with its own status from `list_accounts`, so the spend gate still holds a `payment_required` or pending account. After `AccountSeparate` or `AccountMatched`, the natural key keeps naming the old account until you call `client.accounts.use(key, account_id)`. Which account carries new buys stays your explicit choice. - **Not carried over.** `AccountSeparate.not_carried_over` lists the account-level configurations the new account lacks. `subscriptions_not_covering` lists the `account_ids`-scoped reporting subscriptions that will not see it. - **Next step.** `next_step(outcome)` is `none` for `AccountUpdated`, `AccountSeparate` and `AccountMatched`. diff --git a/src/adcp/accounts.py b/src/adcp/accounts.py index 7a73b6ca1..95d8a6f02 100644 --- a/src/adcp/accounts.py +++ b/src/adcp/accounts.py @@ -584,26 +584,38 @@ async def change( raise failure raise AssertionError("unreachable") # pragma: no cover - async def use(self, key: AccountReference | Mapping[str, Any], account_id: str) -> None: + async def use( + self, + key: AccountReference | Mapping[str, Any], + account_id: str, + *, + status: str | None = None, + ) -> None: """Point a natural key at ``account_id``: new buys for ``key`` go to that account. The explicit step after :meth:`change` returned :class:`~adcp.account_outcomes.AccountSeparate` or :class:`~adcp.account_outcomes.AccountMatched`; the registry never - re-points on its own. + re-points on its own. The account's ``status`` is read from + ``list_accounts`` unless you pass it, so the spend gate still holds a + ``payment_required`` or pending account; an account the caller cannot + list is recorded without a status and refused for spend until it is. """ ref = account_key_payload(TypeAdapter(AccountReference).validate_python(key)) if "account_id" in ref: raise ValueError("use() maps a buyer-declared natural key, not an account_id") if not account_id: raise ValueError("use() needs the account_id new buys go to") + if status is None: + listed = await self._read_account(account_id) + status = None if listed is None else listed.get("status") previous = await self.get(ref) await self._storage.save( self._seller, account_key(ref), AccountRecord( account_id=account_id, - status="active", + status=None if status is None else str(status), wire_key=None if previous is None else previous.wire_key, wire_version=None if previous is None else previous.wire_version, ), diff --git a/tests/test_account_change.py b/tests/test_account_change.py index b12fc6d03..3cf471dbf 100644 --- a/tests/test_account_change.py +++ b/tests/test_account_change.py @@ -126,7 +126,7 @@ async def test_a_separate_account_lists_what_is_missing_and_never_repoints() -> account={"account_id": "acc-2", "name": "Acme BE", "status": "active"}, ) ) - await client.accounts.use(KEY, "acc-1") + await client.accounts.use(KEY, "acc-1", status="active") outcome = await client.accounts.change( "acc-1", {"brand": {"domain": "acme.example", "countries": ["BE"]}}, @@ -139,8 +139,15 @@ async def test_a_separate_account_lists_what_is_missing_and_never_repoints() -> assert client.adapter.request_account.call_args.args[0]["allow_separate_account"] is True # Nothing moved: the key still names the old account until use(). assert (await client.accounts.get(KEY)).account_id == "acc-1" + client.adapter.list_accounts = AsyncMock( + return_value=_completed( + {"accounts": [{"account_id": "acc-2", "name": "Acme BE", "status": "payment_required"}]} + ) + ) await client.accounts.use(outcome.key, outcome.account_id) - assert (await client.accounts.get(KEY)).account_id == "acc-2" + record = await client.accounts.get(KEY) + # The new account's own status is kept, so the spend gate still applies. + assert (record.account_id, record.status) == ("acc-2", "payment_required") async def test_matched_pending_and_rejected_outcomes() -> None: From 539d829b2d01c10216b4e7fddd5262aa39f4a938 Mon Sep 17 00:00:00 2001 From: Brian O'Kelley Date: Sun, 1 Nov 2026 18:38:12 +0000 Subject: [PATCH 4/6] feat(accounts): read a pending account change back with change_status change returns a pending change without keeping it in the registry, which maps natural keys rather than account ids. change_status(account_id, request_id) reads the request back through list_accounts and returns its current outcome. Co-Authored-By: Claude Opus 5.5 (1M context) --- docs/accounts-ensure-and-change.md | 1 + src/adcp/accounts.py | 21 +++++++++++++++++++++ tests/test_account_change.py | 26 ++++++++++++++++++++++++++ 3 files changed, 48 insertions(+) diff --git a/docs/accounts-ensure-and-change.md b/docs/accounts-ensure-and-change.md index 8ac773a05..c7bbecae2 100644 --- a/docs/accounts-ensure-and-change.md +++ b/docs/accounts-ensure-and-change.md @@ -41,6 +41,7 @@ match outcome: ``` - **Default.** `allow_separate_account` defaults to `False`, as in the spec. A seller that would accept the change only as a separate account rejects it, and `outcome.requires_new_account` is `True` (`ACCOUNT_CHANGE_REQUIRES_NEW_ACCOUNT`). +- **Pending changes.** A pending change is not kept in the registry. Keep its `request_id` and read it back with `await client.accounts.change_status(account_id, request_id)`; a new change request supersedes an open one. - **No silent re-point.** `change()` never re-points the registry; `use()` records the account with its own status from `list_accounts`, so the spend gate still holds a `payment_required` or pending account. After `AccountSeparate` or `AccountMatched`, the natural key keeps naming the old account until you call `client.accounts.use(key, account_id)`. Which account carries new buys stays your explicit choice. - **Not carried over.** `AccountSeparate.not_carried_over` lists the account-level configurations the new account lacks. `subscriptions_not_covering` lists the `account_ids`-scoped reporting subscriptions that will not see it. - **Next step.** `next_step(outcome)` is `none` for `AccountUpdated`, `AccountSeparate` and `AccountMatched`. diff --git a/src/adcp/accounts.py b/src/adcp/accounts.py index 95d8a6f02..fe1bcd472 100644 --- a/src/adcp/accounts.py +++ b/src/adcp/accounts.py @@ -512,6 +512,10 @@ async def change( account; the registry is not re-pointed until :meth:`use`), :class:`~adcp.account_outcomes.AccountPending` and :class:`~adcp.account_outcomes.AccountRejected`. + + A pending change is not kept in the registry, which maps natural keys, + not account ids: keep its ``request_id`` and read it back with + :meth:`change_status`. A new change request supersedes an open one. """ payload = { key: _payload(value) if hasattr(value, "model_dump") else value @@ -584,6 +588,23 @@ async def change( raise failure raise AssertionError("unreachable") # pragma: no cover + async def change_status(self, account_id: str, request_id: str) -> AccountChangeOutcome: + """The current outcome of a change request (``list_accounts`` ``request_ids``).""" + found = await self._read_requests([request_id]) + wire = found.get(request_id) + if wire is None: + return AccountRejected( + errors=( + { + "code": "ACCOUNT_REQUEST_NOT_FOUND", + "message": "The seller no longer knows this request; send it again.", + }, + ), + via="request_account", + request_id=request_id, + ) + return await self._change_outcome(account_id, wire) + async def use( self, key: AccountReference | Mapping[str, Any], diff --git a/tests/test_account_change.py b/tests/test_account_change.py index 3cf471dbf..5d1c0a99c 100644 --- a/tests/test_account_change.py +++ b/tests/test_account_change.py @@ -203,3 +203,29 @@ async def test_change_refuses_unknown_fields_and_sellers_without_account_request assert unsupported.value.error_codes == ["ACCOUNT_REQUESTS_UNSUPPORTED"] assert is_pre_dispatch_error(unsupported.value) plain.adapter.request_account.assert_not_called() + + +async def test_a_pending_change_is_read_back_by_request_id() -> None: + client = _client() + client.adapter.list_accounts = AsyncMock( + side_effect=[ + _completed( + { + "accounts": [], + "account_requests": [ + _record( + "resolved", + account_id="acc-1", + resolution="updated", + account={**ACCOUNT, "revision": 4}, + ) + ], + } + ), + _completed({"accounts": [], "account_requests": []}), + ] + ) + resolved = await client.accounts.change_status("acc-1", "req-1") + assert isinstance(resolved, AccountUpdated) and resolved.revision == 4 + gone = await client.accounts.change_status("acc-1", "req-1") + assert isinstance(gone, AccountRejected) From 84c89883efd6c6118d6fc3fb8e1e6413241e7a5f Mon Sep 17 00:00:00 2001 From: Brian O'Kelley Date: Sun, 1 Nov 2026 18:42:03 +0000 Subject: [PATCH 5/6] docs(accounts): add the account change refusals to the error reference Co-Authored-By: Claude Opus 5.5 (1M context) --- docs/reporting-subscriptions-errors.md | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/docs/reporting-subscriptions-errors.md b/docs/reporting-subscriptions-errors.md index 8bd7716ed..652a6ccb7 100644 --- a/docs/reporting-subscriptions-errors.md +++ b/docs/reporting-subscriptions-errors.md @@ -21,3 +21,15 @@ Every SDK error carries `code` and `recovery` (`transient`, `correctable` or `te | `SUBSCRIPTION_ROSTER_UNAVAILABLE` | `ReliableReportingConfigurationError` | n/a | correctable | serve `list_accounts`, or supply `SubscriptionOptions.accounts` | `AccountRequestError` is raised only when `client.accounts.ensure` could not be processed; it carries the wire codes. Account outcomes are values. + +## Accounts + +`client.accounts.change()` refuses locally, before anything is sent, with `AccountRequestError` (`dispatched=False`, `recovery="correctable"`): + +| Code | Cause | Fix | +|---|---|---| +| `ACCOUNT_REVISION_REQUIRED` | a change to `billing`, `billing_entity` or `payment_terms` without `revision` | pass the revision the change was approved against, read from `list_accounts` | +| `ACCOUNT_CHANGE_INVALID` | no attribute, or one `change()` cannot change | change `brand`, `operator_unit`, `currency`, `timezone` or a money term | +| `ACCOUNT_REQUESTS_UNSUPPORTED` | the seller does not advertise `account.requests` | change the account through `sync_accounts` | + +A seller rejection is a value: `AccountRejected`. Its `requires_new_account` is `True` for `ACCOUNT_CHANGE_REQUIRES_NEW_ACCOUNT`; resend with `allow_separate_account=True`. A `CONFLICT` on a money change is raised as `AccountRequestError`, because the human must approve the change against the new revision. From 73373fc536be5f19debc6d8a7a3192c6bc36c522 Mon Sep 17 00:00:00 2001 From: Brian O'Kelley Date: Sun, 1 Nov 2026 19:47:24 +0000 Subject: [PATCH 6/6] style(accounts): sort the adcp account outcome imports Co-Authored-By: Claude Opus 5.5 (1M context) --- src/adcp/__init__.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/adcp/__init__.py b/src/adcp/__init__.py index 3f6ab3ccc..835eb37b3 100644 --- a/src/adcp/__init__.py +++ b/src/adcp/__init__.py @@ -1854,11 +1854,11 @@ def get_adcp_version() -> str: AccountChangeOutcome, AccountMatched, AccountOutcome, - AccountSeparate, - AccountUpdated, AccountPending, AccountReady, AccountRejected, + AccountSeparate, + AccountUpdated, PendingAction, format_pending_action, next_step,