From b72e72e0261f16f89673cf69ca1a315a4944e80f Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 3 May 2026 11:44:50 +0000 Subject: [PATCH 1/2] feat(decisioning): state-machine helpers, typed exceptions, ref_account_id (#455) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds three sets of additive helpers to adcp.decisioning (issue #455, Batch 1 item 3 of the JS 6.7 parity epic #452): (a) State-machine transition helpers - MEDIA_BUY_TRANSITIONS: dict[MediaBuyStatus, set[MediaBuyStatus]] — valid from→{to} moves per spec; terminals map to empty set - validate_media_buy_transition(from, to): raises AdcpError(INVALID_REQUEST) on disallowed transitions or unknown status strings - CREATIVE_TRANSITIONS / validate_creative_transition — same shape; includes the rejected→processing resubmit path and archived→approved unarchive path from creative-status.json (would have been missing in the original proposal) (b) Typed AdcpError subclasses in adcp.decisioning.exceptions PermissionDeniedError, AuthRequiredError, ServiceUnavailableError, RateLimitedError, MediaBuyNotFoundError, AccountNotFoundError, BillingNotPermittedForAgentError, RequestValidationError — each hard-codes the correct wire code and recovery semantics so adopters don't open-code AdcpError(code=..., recovery=...) inline. Named RequestValidationError (not ValidationError) to avoid shadowing pydantic.ValidationError. (c) ref_account_id(ref) -> str | None Extracts account_id from a wire account reference dict; eliminates the ref.get('account_id') if ref else None inline pattern. 56 new tests, all passing. Ruff and mypy clean on new files. https://claude.ai/code/session_01XbBfrSnfmVy6fQtoebgxWJ --- src/adcp/decisioning/__init__.py | 30 ++ src/adcp/decisioning/exceptions.py | 165 +++++++++ src/adcp/decisioning/transitions.py | 223 ++++++++++++ ...test_decisioning_transitions_exceptions.py | 322 ++++++++++++++++++ 4 files changed, 740 insertions(+) create mode 100644 src/adcp/decisioning/exceptions.py create mode 100644 src/adcp/decisioning/transitions.py create mode 100644 tests/test_decisioning_transitions_exceptions.py diff --git a/src/adcp/decisioning/__init__.py b/src/adcp/decisioning/__init__.py index 077299ade..63f9987e4 100644 --- a/src/adcp/decisioning/__init__.py +++ b/src/adcp/decisioning/__init__.py @@ -63,6 +63,16 @@ def create_media_buy( AuthInfo, RequestContext, ) +from adcp.decisioning.exceptions import ( + AccountNotFoundError, + AuthRequiredError, + BillingNotPermittedForAgentError, + MediaBuyNotFoundError, + PermissionDeniedError, + RateLimitedError, + RequestValidationError, + ServiceUnavailableError, +) from adcp.decisioning.mock_ad_server import ( InMemoryMockAdServer, MockAdServer, @@ -129,6 +139,13 @@ def create_media_buy( TaskRegistry, TaskState, ) +from adcp.decisioning.transitions import ( + CREATIVE_TRANSITIONS, + MEDIA_BUY_TRANSITIONS, + ref_account_id, + validate_creative_transition, + validate_media_buy_transition, +) from adcp.decisioning.types import ( Account, AdcpError, @@ -174,9 +191,12 @@ def __init__(self, *args: object, **kwargs: object) -> None: __all__ = [ "Account", + "AccountNotFoundError", "AccountStore", "AdcpError", + "AuthRequiredError", "ApiKeyCredential", + "BillingNotPermittedForAgentError", "AudiencePlatform", "AuditingBuyerAgentRegistry", "AuthInfo", @@ -186,6 +206,7 @@ def __init__(self, *args: object, **kwargs: object) -> None: "BuyerAgentDefaultTerms", "BuyerAgentRegistry", "BuyerAgentStatus", + "CREATIVE_TRANSITIONS", "CachingBuyerAgentRegistry", "CampaignGovernancePlatform", "CollectionList", @@ -202,12 +223,15 @@ def __init__(self, *args: object, **kwargs: object) -> None: "FromAuthAccounts", "GOVERNANCE_SPECIALISMS", "GovernanceContextJWS", + "MEDIA_BUY_TRANSITIONS", + "MediaBuyNotFoundError", "HttpSigCredential", "InMemoryMockAdServer", "InMemoryTaskRegistry", "MaybeAsync", "MockAdServer", "OAuthCredential", + "PermissionDeniedError", "PgTaskRegistry", "PostgresTaskRegistry", "Proposal", @@ -215,10 +239,13 @@ def __init__(self, *args: object, **kwargs: object) -> None: "PropertyListReference", "PropertyListsPlatform", "RateLimitedBuyerAgentRegistry", + "RateLimitedError", "RequestContext", + "RequestValidationError", "ResourceResolver", "SalesPlatform", "SalesResult", + "ServiceUnavailableError", "SignalsPlatform", "SingletonAccounts", "StateReader", @@ -234,7 +261,10 @@ def __init__(self, *args: object, **kwargs: object) -> None: "mixed_registry", "project_account_for_response", "project_business_entity_for_response", + "ref_account_id", "serve", "signing_only_registry", "validate_billing_for_agent", + "validate_creative_transition", + "validate_media_buy_transition", ] diff --git a/src/adcp/decisioning/exceptions.py b/src/adcp/decisioning/exceptions.py new file mode 100644 index 000000000..1f4f6e38b --- /dev/null +++ b/src/adcp/decisioning/exceptions.py @@ -0,0 +1,165 @@ +"""Typed AdcpError subclasses for common rejection patterns. + +Adopters raise these in Platform method bodies instead of constructing +``AdcpError(code=..., recovery=...)`` inline. The framework's dispatcher +catches :class:`~adcp.decisioning.types.AdcpError` at the dispatch seam and +serialises to the wire ``adcp_error`` envelope — these subclasses are caught +by the same handler. + +All subclasses hard-code the correct ``code`` and ``recovery`` values from +the AdCP spec (``schemas/cache/3.0.0/enums/error-code.json``). Keyword +``details`` captures free-form extras; any unknown kwargs are forwarded as the +``details`` dict to :class:`~adcp.decisioning.types.AdcpError`. +""" + +from __future__ import annotations + +from typing import Any + +from adcp.decisioning.types import AdcpError + + +class PermissionDeniedError(AdcpError): + """Raised when the authenticated principal lacks permission for ``action``. + + Maps to wire code ``PERMISSION_DENIED`` with ``recovery='terminal'``. + """ + + def __init__(self, action: str = "", **details: Any) -> None: + msg = f"Permission denied: {action}" if action else "Permission denied" + super().__init__( + "PERMISSION_DENIED", + message=msg, + recovery="terminal", + details=details if details else None, + ) + + +class AuthRequiredError(AdcpError): + """Raised when a request arrives without valid authentication. + + Maps to ``AUTH_REQUIRED`` with ``recovery='correctable'`` (per AdCP 3.0.4 + prose — missing-credentials case; the buyer should re-present credentials + rather than abandon). See the note in ``adcp.server.helpers`` about the + planned 3.1 split into ``AUTH_MISSING`` / ``AUTH_INVALID``. + """ + + def __init__(self, **details: Any) -> None: + super().__init__( + "AUTH_REQUIRED", + message="Authentication required", + recovery="correctable", + details=details if details else None, + ) + + +class ServiceUnavailableError(AdcpError): + """Raised when the seller service is temporarily unavailable. + + Maps to ``SERVICE_UNAVAILABLE`` with ``recovery='transient'``. + """ + + def __init__(self, message: str = "Service temporarily unavailable", **details: Any) -> None: + super().__init__( + "SERVICE_UNAVAILABLE", + message=message, + recovery="transient", + details=details if details else None, + ) + + +class RateLimitedError(AdcpError): + """Raised when the buyer has exceeded the request rate limit. + + Maps to ``RATE_LIMITED`` with ``recovery='transient'``. Pass + ``retry_after`` to tell the buyer how long to wait. + """ + + def __init__(self, *, retry_after: int | None = None, **details: Any) -> None: + super().__init__( + "RATE_LIMITED", + message="Too many requests", + recovery="transient", + retry_after=retry_after, + details=details if details else None, + ) + + +class MediaBuyNotFoundError(AdcpError): + """Raised when the referenced media buy does not exist. + + Maps to ``MEDIA_BUY_NOT_FOUND`` with ``recovery='correctable'``. + """ + + def __init__(self, **details: Any) -> None: + super().__init__( + "MEDIA_BUY_NOT_FOUND", + message="Media buy not found", + recovery="correctable", + details=details if details else None, + ) + + +class AccountNotFoundError(AdcpError): + """Raised when the referenced account does not exist. + + Maps to ``ACCOUNT_NOT_FOUND`` with ``recovery='terminal'``. + """ + + def __init__(self, **details: Any) -> None: + super().__init__( + "ACCOUNT_NOT_FOUND", + message="Account not found", + recovery="terminal", + details=details if details else None, + ) + + +class BillingNotPermittedForAgentError(AdcpError): + """Raised when a buyer agent attempts a billing operation it is not + authorised to perform. + + Maps to ``BILLING_NOT_PERMITTED_FOR_AGENT`` with ``recovery='terminal'``. + """ + + def __init__(self, **details: Any) -> None: + super().__init__( + "BILLING_NOT_PERMITTED_FOR_AGENT", + message="Billing operations are not permitted for this agent", + recovery="terminal", + details=details if details else None, + ) + + +class RequestValidationError(AdcpError): + """Raised when the request fails validation (malformed fields, constraint + violations, etc.). + + Maps to ``VALIDATION_ERROR`` with ``recovery='correctable'``. + + Named ``RequestValidationError`` (not ``ValidationError``) to avoid + shadowing ``pydantic.ValidationError`` and ``adcp.validation.legacy.ValidationError`` + in adopter import namespaces. + """ + + def __init__( + self, message: str = "Request validation failed", **details: Any + ) -> None: + super().__init__( + "VALIDATION_ERROR", + message=message, + recovery="correctable", + details=details if details else None, + ) + + +__all__ = [ + "AccountNotFoundError", + "AuthRequiredError", + "BillingNotPermittedForAgentError", + "MediaBuyNotFoundError", + "PermissionDeniedError", + "RateLimitedError", + "RequestValidationError", + "ServiceUnavailableError", +] diff --git a/src/adcp/decisioning/transitions.py b/src/adcp/decisioning/transitions.py new file mode 100644 index 000000000..19962e5de --- /dev/null +++ b/src/adcp/decisioning/transitions.py @@ -0,0 +1,223 @@ +"""State-machine transition helpers and account-reference utilities. + +Provides: + +* :data:`MEDIA_BUY_TRANSITIONS` — valid ``from → {to}`` state moves for + media buys, keyed on :class:`~adcp.types.MediaBuyStatus`. +* :func:`validate_media_buy_transition` — assert a transition is allowed, + raising :class:`~adcp.decisioning.types.AdcpError` with + ``code='INVALID_REQUEST'`` if not. +* :data:`CREATIVE_TRANSITIONS` — same shape for + :class:`~adcp.types.CreativeStatus`. +* :func:`validate_creative_transition` — same pattern for creatives. +* :func:`ref_account_id` — extract ``account_id`` from a wire account + reference dict without repeating ``ref.get('account_id')`` inline. + +Transition maps are derived from the AdCP 3.0 spec state-machine diagrams: +``schemas/cache/3.0.0/enums/media-buy-status.json`` and +``schemas/cache/3.0.0/enums/creative-status.json``. Terminal states have +empty sets; resubmit / unarchive paths for creatives are included. + +These helpers are distinct from :data:`adcp.server.helpers.MEDIA_BUY_STATE_MACHINE`, +which maps a status to the buyer *actions* available at that status. These +helpers validate *state-to-state transitions* (e.g. that a seller does not +move a media buy from ``completed`` back to ``active``). +""" + +from __future__ import annotations + +from typing import Any + +from adcp.decisioning.types import AdcpError +from adcp.types import CreativeStatus, MediaBuyStatus + +# --------------------------------------------------------------------------- +# Media buy state machine +# --------------------------------------------------------------------------- + +#: Valid from-state → set-of-allowed-to-states for media buys. +#: +#: Terminal states (``completed``, ``rejected``, ``canceled``) map to the +#: empty set — no further transitions are allowed. +MEDIA_BUY_TRANSITIONS: dict[MediaBuyStatus, set[MediaBuyStatus]] = { + MediaBuyStatus.pending_creatives: { + MediaBuyStatus.pending_start, + MediaBuyStatus.canceled, + }, + MediaBuyStatus.pending_start: { + MediaBuyStatus.active, + MediaBuyStatus.canceled, + }, + MediaBuyStatus.active: { + MediaBuyStatus.paused, + MediaBuyStatus.completed, + MediaBuyStatus.canceled, + }, + MediaBuyStatus.paused: { + MediaBuyStatus.active, + MediaBuyStatus.canceled, + }, + MediaBuyStatus.completed: set(), + MediaBuyStatus.rejected: set(), + MediaBuyStatus.canceled: set(), +} + + +def validate_media_buy_transition( + from_state: str | MediaBuyStatus, + to_state: str | MediaBuyStatus, +) -> None: + """Assert that a media buy state transition is allowed by the spec. + + Accepts both ``MediaBuyStatus`` enum members and raw string values (as + they arrive from the wire). Unknown string values raise + ``AdcpError('INVALID_REQUEST')`` rather than a bare ``ValueError`` so + the framework's error envelope is always produced. + + :raises AdcpError: ``code='INVALID_REQUEST'``, ``recovery='correctable'`` + when the transition is not in :data:`MEDIA_BUY_TRANSITIONS` or when + either state string is not a recognised :class:`MediaBuyStatus` value. + """ + if isinstance(from_state, str): + try: + from_s = MediaBuyStatus(from_state) + except ValueError: + raise AdcpError( + "INVALID_REQUEST", + message=f"Unknown media buy status: {from_state!r}", + recovery="correctable", + ) + else: + from_s = from_state + + if isinstance(to_state, str): + try: + to_s = MediaBuyStatus(to_state) + except ValueError: + raise AdcpError( + "INVALID_REQUEST", + message=f"Unknown media buy status: {to_state!r}", + recovery="correctable", + ) + else: + to_s = to_state + + allowed = MEDIA_BUY_TRANSITIONS.get(from_s, set()) + if to_s not in allowed: + raise AdcpError( + "INVALID_REQUEST", + message=( + f"Media buy transition {from_s.value!r} → {to_s.value!r} is not allowed" + ), + recovery="correctable", + ) + + +# --------------------------------------------------------------------------- +# Creative asset state machine +# --------------------------------------------------------------------------- + +#: Valid from-state → set-of-allowed-to-states for creative assets. +#: +#: ``rejected`` allows resubmission back to ``processing`` (the buyer may fix +#: the creative and call ``sync_creatives`` again). ``archived`` allows +#: unarchiving back to ``approved``. +CREATIVE_TRANSITIONS: dict[CreativeStatus, set[CreativeStatus]] = { + CreativeStatus.processing: { + CreativeStatus.pending_review, + CreativeStatus.rejected, + }, + CreativeStatus.pending_review: { + CreativeStatus.approved, + CreativeStatus.rejected, + }, + CreativeStatus.approved: { + CreativeStatus.archived, + }, + CreativeStatus.rejected: { + CreativeStatus.processing, # resubmit via sync_creatives + }, + CreativeStatus.archived: { + CreativeStatus.approved, # unarchive path + }, +} + + +def validate_creative_transition( + from_state: str | CreativeStatus, + to_state: str | CreativeStatus, +) -> None: + """Assert that a creative asset state transition is allowed by the spec. + + Accepts both :class:`~adcp.types.CreativeStatus` enum members and raw + string values. Unknown string values raise ``AdcpError('INVALID_REQUEST')`` + so the error is always wire-shaped. + + :raises AdcpError: ``code='INVALID_REQUEST'``, ``recovery='correctable'`` + when the transition is not in :data:`CREATIVE_TRANSITIONS` or when + either state string is not a recognised :class:`CreativeStatus` value. + """ + if isinstance(from_state, str): + try: + from_s = CreativeStatus(from_state) + except ValueError: + raise AdcpError( + "INVALID_REQUEST", + message=f"Unknown creative status: {from_state!r}", + recovery="correctable", + ) + else: + from_s = from_state + + if isinstance(to_state, str): + try: + to_s = CreativeStatus(to_state) + except ValueError: + raise AdcpError( + "INVALID_REQUEST", + message=f"Unknown creative status: {to_state!r}", + recovery="correctable", + ) + else: + to_s = to_state + + allowed = CREATIVE_TRANSITIONS.get(from_s, set()) + if to_s not in allowed: + raise AdcpError( + "INVALID_REQUEST", + message=( + f"Creative transition {from_s.value!r} → {to_s.value!r} is not allowed" + ), + recovery="correctable", + ) + + +# --------------------------------------------------------------------------- +# Account reference utilities +# --------------------------------------------------------------------------- + + +def ref_account_id(ref: dict[str, Any] | None) -> str | None: + """Extract ``account_id`` from a wire account reference dict. + + Eliminates the ``ref.get('account_id') if ref else None`` pattern that + appears in multi-tenant platform methods. + + :param ref: An account reference dict as received from the wire, or + ``None`` (for requests that carry no account reference). + :returns: The ``account_id`` string if present and a ``str``, else + ``None``. + """ + if ref is None: + return None + account_id = ref.get("account_id") + return account_id if isinstance(account_id, str) else None + + +__all__ = [ + "CREATIVE_TRANSITIONS", + "MEDIA_BUY_TRANSITIONS", + "ref_account_id", + "validate_creative_transition", + "validate_media_buy_transition", +] diff --git a/tests/test_decisioning_transitions_exceptions.py b/tests/test_decisioning_transitions_exceptions.py new file mode 100644 index 000000000..01898a429 --- /dev/null +++ b/tests/test_decisioning_transitions_exceptions.py @@ -0,0 +1,322 @@ +"""Tests for adcp.decisioning state-machine helpers and typed exceptions. + +Covers: +* MEDIA_BUY_TRANSITIONS / validate_media_buy_transition +* CREATIVE_TRANSITIONS / validate_creative_transition +* ref_account_id +* All eight typed AdcpError subclasses: error codes, recovery semantics +""" + +from __future__ import annotations + +import pytest + +from adcp.decisioning import ( + CREATIVE_TRANSITIONS, + MEDIA_BUY_TRANSITIONS, + AccountNotFoundError, + AdcpError, + AuthRequiredError, + BillingNotPermittedForAgentError, + MediaBuyNotFoundError, + PermissionDeniedError, + RateLimitedError, + RequestValidationError, + ServiceUnavailableError, + ref_account_id, + validate_creative_transition, + validate_media_buy_transition, +) +from adcp.types import CreativeStatus, MediaBuyStatus + +# --------------------------------------------------------------------------- +# MEDIA_BUY_TRANSITIONS map shape +# --------------------------------------------------------------------------- + + +def test_media_buy_transitions_covers_all_statuses() -> None: + assert set(MEDIA_BUY_TRANSITIONS.keys()) == set(MediaBuyStatus) + + +def test_media_buy_transitions_terminal_states_are_empty() -> None: + for terminal in ( + MediaBuyStatus.completed, + MediaBuyStatus.rejected, + MediaBuyStatus.canceled, + ): + assert MEDIA_BUY_TRANSITIONS[terminal] == set(), f"{terminal} should be terminal" + + +def test_media_buy_transitions_active_can_pause_complete_cancel() -> None: + allowed = MEDIA_BUY_TRANSITIONS[MediaBuyStatus.active] + assert MediaBuyStatus.paused in allowed + assert MediaBuyStatus.completed in allowed + assert MediaBuyStatus.canceled in allowed + + +# --------------------------------------------------------------------------- +# validate_media_buy_transition — valid paths +# --------------------------------------------------------------------------- + + +@pytest.mark.parametrize( + "from_state, to_state", + [ + ("pending_creatives", "pending_start"), + ("pending_creatives", "canceled"), + ("pending_start", "active"), + ("pending_start", "canceled"), + ("active", "paused"), + ("active", "completed"), + ("active", "canceled"), + ("paused", "active"), + ("paused", "canceled"), + ], +) +def test_validate_media_buy_transition_valid_strings(from_state: str, to_state: str) -> None: + validate_media_buy_transition(from_state, to_state) # must not raise + + +def test_validate_media_buy_transition_valid_enum_members() -> None: + validate_media_buy_transition(MediaBuyStatus.active, MediaBuyStatus.paused) + + +# --------------------------------------------------------------------------- +# validate_media_buy_transition — invalid paths +# --------------------------------------------------------------------------- + + +@pytest.mark.parametrize( + "from_state, to_state", + [ + ("completed", "active"), + ("rejected", "pending_start"), + ("canceled", "active"), + ("paused", "completed"), # paused cannot complete directly + ("pending_creatives", "active"), # must go through pending_start + ], +) +def test_validate_media_buy_transition_raises_on_disallowed( + from_state: str, to_state: str +) -> None: + with pytest.raises(AdcpError) as exc_info: + validate_media_buy_transition(from_state, to_state) + err = exc_info.value + assert err.code == "INVALID_REQUEST" + assert err.recovery == "correctable" + + +def test_validate_media_buy_transition_unknown_from_state() -> None: + with pytest.raises(AdcpError) as exc_info: + validate_media_buy_transition("totally_unknown", "active") + assert exc_info.value.code == "INVALID_REQUEST" + + +def test_validate_media_buy_transition_unknown_to_state() -> None: + with pytest.raises(AdcpError) as exc_info: + validate_media_buy_transition("active", "totally_unknown") + assert exc_info.value.code == "INVALID_REQUEST" + + +# --------------------------------------------------------------------------- +# CREATIVE_TRANSITIONS map shape +# --------------------------------------------------------------------------- + + +def test_creative_transitions_covers_all_statuses() -> None: + assert set(CREATIVE_TRANSITIONS.keys()) == set(CreativeStatus) + + +def test_creative_rejected_allows_resubmit() -> None: + assert CreativeStatus.processing in CREATIVE_TRANSITIONS[CreativeStatus.rejected] + + +def test_creative_archived_allows_unarchive() -> None: + assert CreativeStatus.approved in CREATIVE_TRANSITIONS[CreativeStatus.archived] + + +# --------------------------------------------------------------------------- +# validate_creative_transition — valid paths +# --------------------------------------------------------------------------- + + +@pytest.mark.parametrize( + "from_state, to_state", + [ + ("processing", "pending_review"), + ("processing", "rejected"), + ("pending_review", "approved"), + ("pending_review", "rejected"), + ("approved", "archived"), + ("rejected", "processing"), # resubmit + ("archived", "approved"), # unarchive + ], +) +def test_validate_creative_transition_valid(from_state: str, to_state: str) -> None: + validate_creative_transition(from_state, to_state) # must not raise + + +def test_validate_creative_transition_valid_enum_members() -> None: + validate_creative_transition(CreativeStatus.processing, CreativeStatus.pending_review) + + +# --------------------------------------------------------------------------- +# validate_creative_transition — invalid paths +# --------------------------------------------------------------------------- + + +@pytest.mark.parametrize( + "from_state, to_state", + [ + ("approved", "processing"), # cannot go backwards + ("pending_review", "processing"), + ("archived", "processing"), # archived only unarchives to approved + ], +) +def test_validate_creative_transition_raises_on_disallowed( + from_state: str, to_state: str +) -> None: + with pytest.raises(AdcpError) as exc_info: + validate_creative_transition(from_state, to_state) + err = exc_info.value + assert err.code == "INVALID_REQUEST" + assert err.recovery == "correctable" + + +def test_validate_creative_transition_unknown_from_state() -> None: + with pytest.raises(AdcpError) as exc_info: + validate_creative_transition("mystery_state", "approved") + assert exc_info.value.code == "INVALID_REQUEST" + + +# --------------------------------------------------------------------------- +# ref_account_id +# --------------------------------------------------------------------------- + + +def test_ref_account_id_extracts_string() -> None: + assert ref_account_id({"account_id": "acct_123"}) == "acct_123" + + +def test_ref_account_id_none_input() -> None: + assert ref_account_id(None) is None + + +def test_ref_account_id_missing_key() -> None: + assert ref_account_id({"something_else": "x"}) is None + + +def test_ref_account_id_non_string_value() -> None: + assert ref_account_id({"account_id": 42}) is None # type: ignore[dict-item] + + +def test_ref_account_id_empty_dict() -> None: + assert ref_account_id({}) is None + + +# --------------------------------------------------------------------------- +# Typed exception classes +# --------------------------------------------------------------------------- + + +def test_permission_denied_error_code_and_recovery() -> None: + err = PermissionDeniedError("update_media_buy") + assert err.code == "PERMISSION_DENIED" + assert err.recovery == "terminal" + assert "update_media_buy" in str(err) + + +def test_permission_denied_error_no_action() -> None: + err = PermissionDeniedError() + assert err.code == "PERMISSION_DENIED" + assert "Permission denied" in str(err) + + +def test_auth_required_error() -> None: + err = AuthRequiredError() + assert err.code == "AUTH_REQUIRED" + assert err.recovery == "correctable" + + +def test_service_unavailable_error() -> None: + err = ServiceUnavailableError() + assert err.code == "SERVICE_UNAVAILABLE" + assert err.recovery == "transient" + + +def test_service_unavailable_error_custom_message() -> None: + err = ServiceUnavailableError("DB is down") + assert "DB is down" in str(err) + + +def test_rate_limited_error() -> None: + err = RateLimitedError() + assert err.code == "RATE_LIMITED" + assert err.recovery == "transient" + assert err.retry_after is None + + +def test_rate_limited_error_retry_after() -> None: + err = RateLimitedError(retry_after=30) + assert err.retry_after == 30 + + +def test_media_buy_not_found_error() -> None: + err = MediaBuyNotFoundError() + assert err.code == "MEDIA_BUY_NOT_FOUND" + assert err.recovery == "correctable" + + +def test_account_not_found_error() -> None: + err = AccountNotFoundError() + assert err.code == "ACCOUNT_NOT_FOUND" + assert err.recovery == "terminal" + + +def test_billing_not_permitted_for_agent_error() -> None: + err = BillingNotPermittedForAgentError() + assert err.code == "BILLING_NOT_PERMITTED_FOR_AGENT" + assert err.recovery == "terminal" + + +def test_request_validation_error() -> None: + err = RequestValidationError() + assert err.code == "VALIDATION_ERROR" + assert err.recovery == "correctable" + + +def test_request_validation_error_custom_message() -> None: + err = RequestValidationError("budget field is required") + assert "budget field is required" in str(err) + + +def test_typed_exceptions_are_adcp_error_subclasses() -> None: + for cls in ( + PermissionDeniedError, + AuthRequiredError, + ServiceUnavailableError, + RateLimitedError, + MediaBuyNotFoundError, + AccountNotFoundError, + BillingNotPermittedForAgentError, + RequestValidationError, + ): + assert issubclass(cls, AdcpError), f"{cls.__name__} must subclass AdcpError" + + +def test_typed_exceptions_wire_projection() -> None: + err = RateLimitedError(retry_after=60) + wire = err.to_wire() + assert wire["code"] == "RATE_LIMITED" + assert wire["recovery"] == "transient" + assert wire["retry_after"] == 60 + + +def test_typed_exception_details_forwarded() -> None: + err = PermissionDeniedError("read", reason="suspended") + assert err.details == {"reason": "suspended"} + + +def test_typed_exception_no_details_is_empty() -> None: + err = AccountNotFoundError() + assert err.details == {} From fdaded6189f5cc9dd1f60fb653b5de14f0af057e Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 3 May 2026 11:46:42 +0000 Subject: [PATCH 2/2] fix(decisioning): correct recovery for PERMISSION_DENIED and BILLING_NOT_PERMITTED_FOR_AGENT Both codes classify as 'correctable' in the spec's enumMetadata block (schemas/cache/enums/error-code.json), not 'terminal'. The initial implementation used the wrong recovery value; this aligns with the authoritative enumMetadata classification that SDKs MUST consume. https://claude.ai/code/session_01XbBfrSnfmVy6fQtoebgxWJ --- src/adcp/decisioning/exceptions.py | 13 +++++++++---- tests/test_decisioning_transitions_exceptions.py | 4 ++-- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/src/adcp/decisioning/exceptions.py b/src/adcp/decisioning/exceptions.py index 1f4f6e38b..c86c5e02d 100644 --- a/src/adcp/decisioning/exceptions.py +++ b/src/adcp/decisioning/exceptions.py @@ -22,7 +22,10 @@ class PermissionDeniedError(AdcpError): """Raised when the authenticated principal lacks permission for ``action``. - Maps to wire code ``PERMISSION_DENIED`` with ``recovery='terminal'``. + Maps to wire code ``PERMISSION_DENIED`` with ``recovery='correctable'`` + (spec ``enumMetadata`` classification — the request can be retried after + the underlying permission is resolved, e.g. minting a valid governance + token or contacting the seller). """ def __init__(self, action: str = "", **details: Any) -> None: @@ -30,7 +33,7 @@ def __init__(self, action: str = "", **details: Any) -> None: super().__init__( "PERMISSION_DENIED", message=msg, - recovery="terminal", + recovery="correctable", details=details if details else None, ) @@ -119,14 +122,16 @@ class BillingNotPermittedForAgentError(AdcpError): """Raised when a buyer agent attempts a billing operation it is not authorised to perform. - Maps to ``BILLING_NOT_PERMITTED_FOR_AGENT`` with ``recovery='terminal'``. + Maps to ``BILLING_NOT_PERMITTED_FOR_AGENT`` with ``recovery='correctable'`` + (spec ``enumMetadata`` classification — retry with a permitted billing value + from ``error.details.suggested_billing``, or surface to a human when absent). """ def __init__(self, **details: Any) -> None: super().__init__( "BILLING_NOT_PERMITTED_FOR_AGENT", message="Billing operations are not permitted for this agent", - recovery="terminal", + recovery="correctable", details=details if details else None, ) diff --git a/tests/test_decisioning_transitions_exceptions.py b/tests/test_decisioning_transitions_exceptions.py index 01898a429..203245608 100644 --- a/tests/test_decisioning_transitions_exceptions.py +++ b/tests/test_decisioning_transitions_exceptions.py @@ -222,7 +222,7 @@ def test_ref_account_id_empty_dict() -> None: def test_permission_denied_error_code_and_recovery() -> None: err = PermissionDeniedError("update_media_buy") assert err.code == "PERMISSION_DENIED" - assert err.recovery == "terminal" + assert err.recovery == "correctable" assert "update_media_buy" in str(err) @@ -276,7 +276,7 @@ def test_account_not_found_error() -> None: def test_billing_not_permitted_for_agent_error() -> None: err = BillingNotPermittedForAgentError() assert err.code == "BILLING_NOT_PERMITTED_FOR_AGENT" - assert err.recovery == "terminal" + assert err.recovery == "correctable" def test_request_validation_error() -> None: