diff --git a/src/adcp/decisioning/proposal_dispatch.py b/src/adcp/decisioning/proposal_dispatch.py index cf86d9655..1d2eda1c1 100644 --- a/src/adcp/decisioning/proposal_dispatch.py +++ b/src/adcp/decisioning/proposal_dispatch.py @@ -51,7 +51,7 @@ import contextvars import functools import logging -from datetime import datetime, timezone +from datetime import datetime, timedelta, timezone from typing import TYPE_CHECKING, Any, cast from adcp.decisioning.proposal_lifecycle import ( @@ -417,8 +417,17 @@ async def maybe_persist_draft_after_get_products( * Response carries no ``proposals[]`` (catalog mode). * No recipes attached to products (v1 ``implementation_config`` flow without typed recipes). + + Per #723: when the manager declares + :attr:`ProposalCapabilities.auto_commit_on_put_draft`, the + framework calls :meth:`ProposalStore.commit` immediately after + :meth:`put_draft` to promote ``DRAFT → COMMITTED`` so the + subsequent ``create_media_buy(proposal_id=X)`` can call + ``try_reserve_consumption`` without a separate finalize step. + Used by managers issuing directly-consumable proposals from + ``get_products``. """ - _, store = _resolve_manager_and_store(platform, ctx) + manager, store = _resolve_manager_and_store(platform, ctx) if store is None: return @@ -428,6 +437,14 @@ async def maybe_persist_draft_after_get_products( products = _extract_list(response, "products") or [] + # #723: cache the capability flag once per call — avoid attribute + # lookups inside the per-proposal loop. ``manager`` may legitimately + # be ``None`` (catalog-mode adopter wired a ProposalStore without + # a ProposalManager); in that case auto-commit is off by default. + caps = getattr(manager, "capabilities", None) if manager is not None else None + auto_commit = bool(getattr(caps, "auto_commit_on_put_draft", False)) + auto_commit_ttl = int(getattr(caps, "auto_commit_ttl_seconds", 7 * 24 * 3600)) + for proposal in proposals: proposal_id = _read(proposal, "proposal_id") if proposal_id is None: @@ -453,6 +470,19 @@ async def maybe_persist_draft_after_get_products( account_id=ctx.account.id, recipes_count=len(recipes), ) + if auto_commit: + # Promote DRAFT → COMMITTED in the same dispatch so the + # next call's ``try_reserve_consumption`` finds a COMMITTED + # record. The manager's ``auto_commit_ttl_seconds`` sets + # the expires_at horizon. + expires_at = datetime.now(timezone.utc) + timedelta(seconds=auto_commit_ttl) + await _await_maybe( + store.commit( + str(proposal_id), + expires_at=expires_at, + proposal_payload=proposal_payload, + ) + ) def _collect_recipes_from_products( diff --git a/src/adcp/decisioning/proposal_manager.py b/src/adcp/decisioning/proposal_manager.py index d6247de89..60fb1cec4 100644 --- a/src/adcp/decisioning/proposal_manager.py +++ b/src/adcp/decisioning/proposal_manager.py @@ -137,6 +137,26 @@ class ProposalCapabilities: Kevel for non-guaranteed-remnant in the same proposal). Informational in v1; the per-recipe-kind routing lands in a subsequent PR alongside the typed-recipe registry. + :param auto_commit_on_put_draft: Opt-in shortcut for managers + that issue directly-consumable proposals from ``get_products`` + without a separate ``finalize_proposal`` step. When ``True``, + the framework calls :meth:`ProposalStore.commit` immediately + after :meth:`ProposalStore.put_draft` on every proposal + returned, promoting ``DRAFT → COMMITTED`` so that + ``create_media_buy(proposal_id=X)`` can call + ``try_reserve_consumption`` without a separate buyer round-trip. + Mutually exclusive with ``finalize=True`` (finalize is the + explicit lifecycle; auto-commit is the bypass). Adopters + wiring their own commit lifecycle (e.g. webhook-driven + approval) leave this ``False``. See #723. + :param auto_commit_ttl_seconds: TTL applied to the auto-committed + proposal's ``expires_at``. Used only when + :attr:`auto_commit_on_put_draft` is ``True``. Defaults to + ``604800`` (7 days), matching the salesagent's adopter + choice. Tune up for long-running RFPs; tune down for + spot-market flows. Cap is enforced soft (a warning fires for + values > 30 days) — buyers retrying past the TTL get + ``PROPOSAL_EXPIRED`` and re-request the brief. """ sales_specialism: SalesSpecialism @@ -152,6 +172,8 @@ class ProposalCapabilities: # the framework no longer reads this field. Stops appearing on new # adopter declarations; v1.6+ removes it entirely. multi_decisioning: bool = False + auto_commit_on_put_draft: bool = False + auto_commit_ttl_seconds: int = 7 * 24 * 3600 # 7 days, salesagent default def __post_init__(self) -> None: # Spec only allows the two slugs at v1. Adopter passing a @@ -185,6 +207,87 @@ def __post_init__(self) -> None: recovery="terminal", field="expires_at_grace_seconds", ) + # #723: auto-commit and finalize are mutually exclusive + # lifecycles. ``finalize=True`` says "buyer drives DRAFT → + # COMMITTED explicitly"; ``auto_commit_on_put_draft=True`` says + # "framework promotes on put_draft so no explicit step is + # needed." Both on at once produces a state-machine race + # (the framework auto-commits, then the buyer's finalize call + # rejects because the proposal is no longer DRAFT). Loud-fail + # at construction. + if self.auto_commit_on_put_draft and self.finalize: + raise AdcpError( + "INVALID_REQUEST", + message=( + "ProposalCapabilities: auto_commit_on_put_draft=True and " + "finalize=True are mutually exclusive. auto-commit " + "skips the explicit finalize step (proposals from " + "get_products are committed-on-issuance); finalize " + "requires the buyer to drive the transition. Pick one. " + "See #723." + ), + recovery="terminal", + field="auto_commit_on_put_draft", + ) + # #723 product safety: auto-commit on guaranteed-direct issues + # a silent inventory hold on every ``get_products`` call. GAM / + # ad-server proposal lifecycles require explicit reservation + # precisely because trafficking ops won't accept silent holds + # — a 7-day default TTL would burn inventory across thousands + # of catalog probes per day. Loud-fail; adopters who need + # auto-commit on guaranteed-direct can re-evaluate the + # commercial commitment by wiring the explicit ``finalize`` + # path instead. + if self.auto_commit_on_put_draft and self.sales_specialism == "sales-guaranteed": + raise AdcpError( + "INVALID_REQUEST", + message=( + "ProposalCapabilities: auto_commit_on_put_draft=True is " + "not permitted on sales_specialism='sales-guaranteed'. " + "Auto-commit issues a silent inventory hold on every " + "get_products call (7-day default TTL); guaranteed-" + "direct flows require explicit buyer-driven reservation " + "via the finalize=True lifecycle to avoid unintended " + "commitments. Either switch to " + "sales_specialism='sales-non-guaranteed' (catalog / " + "spot-market flows where auto-commit is appropriate) " + "or set finalize=True instead." + ), + recovery="terminal", + field="auto_commit_on_put_draft", + ) + if self.auto_commit_ttl_seconds <= 0: + raise AdcpError( + "INVALID_REQUEST", + message=( + "ProposalCapabilities.auto_commit_ttl_seconds must be " + f"> 0; got {self.auto_commit_ttl_seconds!r}. Zero or " + "negative TTL would mark proposals expired on commit, " + "making every consumption attempt fail with " + "PROPOSAL_EXPIRED." + ), + recovery="terminal", + field="auto_commit_ttl_seconds", + ) + # Soft-cap warning: a TTL longer than 30 days holds inventory + # for an entire month per catalog probe. Operators can extend + # for long-running RFP flows, but the SDK surfaces a heads-up + # so the default doesn't drift past what the adopter intended. + _soft_cap_seconds = 30 * 24 * 3600 + if self.auto_commit_on_put_draft and self.auto_commit_ttl_seconds > _soft_cap_seconds: + import warnings as _warnings + + _warnings.warn( + f"ProposalCapabilities.auto_commit_ttl_seconds=" + f"{self.auto_commit_ttl_seconds} exceeds the soft cap of " + f"{_soft_cap_seconds} (30 days). Auto-committed proposals " + "hold inventory for the full TTL — verify your commercial " + "model supports holds this long. The framework permits " + "it; this warning fires once per declaration site so the " + "choice is visible at boot.", + UserWarning, + stacklevel=3, + ) @dataclass(frozen=True) diff --git a/tests/test_proposal_auto_commit.py b/tests/test_proposal_auto_commit.py new file mode 100644 index 000000000..374aef594 --- /dev/null +++ b/tests/test_proposal_auto_commit.py @@ -0,0 +1,288 @@ +"""Unit tests for ``ProposalCapabilities.auto_commit_on_put_draft`` (#723). + +When a manager declares ``auto_commit_on_put_draft=True``, the framework +calls :meth:`ProposalStore.commit` immediately after +:meth:`ProposalStore.put_draft` on every proposal returned from +``get_products`` / ``refine_products``. Promotes ``DRAFT → COMMITTED`` +in a single dispatch so a subsequent ``create_media_buy(proposal_id=X)`` +can call ``try_reserve_consumption`` without a separate buyer finalize +round-trip. + +Pre-#723 the storyboard +``media_buy_seller/proposal_finalize/create_media_buy`` was unreachable +for managers that didn't declare ``finalize=True``: brief mode returned +proposals as DRAFT, the next call's ``try_reserve_consumption`` raised +``PROPOSAL_NOT_COMMITTED``. Adopters worked around it by writing +``state=COMMITTED`` directly in ``put_draft`` (salesagent PR #390); this +capability ships the framework-side equivalent so the Protocol surface +stays canonical. +""" + +from __future__ import annotations + +from datetime import datetime, timezone +from typing import Any + +import pytest + +from adcp.decisioning import AdcpError, ProposalCapabilities +from adcp.decisioning.context import RequestContext +from adcp.decisioning.proposal_dispatch import maybe_persist_draft_after_get_products +from adcp.decisioning.proposal_store import InMemoryProposalStore, ProposalState +from adcp.decisioning.types import Account + + +def _account_with_tenant(tenant_id: str = "t1") -> Account: + return Account(id="acct-1", metadata={"tenant_id": tenant_id}) + + +def _ctx(account: Account | None = None) -> RequestContext[Any]: + return RequestContext(account=account or _account_with_tenant()) + + +class _RouterLike: + """Minimal router-shaped platform: exposes the two duck-typed + accessors ``proposal_dispatch`` walks via ``hasattr``. Lets us + exercise ``maybe_persist_draft_after_get_products`` without standing + up a full ``LazyPlatformRouter``.""" + + def __init__( + self, + *, + tenant_id: str = "t1", + manager: Any | None = None, + store: InMemoryProposalStore | None = None, + ) -> None: + self._tenant_id = tenant_id + self._manager = manager + self._store = store + + def proposal_manager_for_tenant(self, tenant_id: str) -> Any | None: + return self._manager if tenant_id == self._tenant_id else None + + def proposal_store_for_tenant(self, tenant_id: str) -> InMemoryProposalStore | None: + return self._store if tenant_id == self._tenant_id else None + + +class _AutoCommitManager: + """Manager declaring auto_commit_on_put_draft=True. The body is + irrelevant — only ``.capabilities`` is consulted by + ``maybe_persist_draft_after_get_products``.""" + + def __init__(self, ttl_seconds: int = 7 * 24 * 3600) -> None: + self.capabilities = ProposalCapabilities( + sales_specialism="sales-non-guaranteed", + auto_commit_on_put_draft=True, + auto_commit_ttl_seconds=ttl_seconds, + ) + + +class _DraftOnlyManager: + """Default manager — leaves proposals DRAFT (the pre-#723 behavior).""" + + capabilities = ProposalCapabilities(sales_specialism="sales-non-guaranteed") + + +# ----- happy path ------------------------------------------------------- + + +@pytest.mark.asyncio +async def test_auto_commit_promotes_draft_to_committed() -> None: + """The whole point of the capability: after + ``maybe_persist_draft_after_get_products`` returns, the store's + record is COMMITTED, not DRAFT. The buyer's subsequent + ``create_media_buy(proposal_id=X)`` can call + ``try_reserve_consumption`` without a finalize round-trip.""" + store = InMemoryProposalStore() + platform = _RouterLike(manager=_AutoCommitManager(), store=store) + + response = { + "products": [], + "proposals": [{"proposal_id": "p1"}], + } + await maybe_persist_draft_after_get_products(platform, response, _ctx()) + + record = await store.get("p1") + assert record is not None + assert record.state is ProposalState.COMMITTED + + +@pytest.mark.asyncio +async def test_auto_commit_off_leaves_record_in_draft() -> None: + """Default capability (``auto_commit_on_put_draft=False``) preserves + the pre-#723 behavior: proposals land as DRAFT and the buyer must + drive the transition via finalize. Regression guard against the + capability defaulting to True or the dispatch path firing + unconditionally.""" + store = InMemoryProposalStore() + platform = _RouterLike(manager=_DraftOnlyManager(), store=store) + + response = {"products": [], "proposals": [{"proposal_id": "p1"}]} + await maybe_persist_draft_after_get_products(platform, response, _ctx()) + + record = await store.get("p1") + assert record is not None + assert record.state is ProposalState.DRAFT + + +@pytest.mark.asyncio +async def test_auto_commit_expires_at_uses_capability_ttl() -> None: + """``auto_commit_ttl_seconds`` controls the COMMITTED record's + ``expires_at``. Default is 7 days; override via the capability + field for spot-market (shorter) or long-running RFP (longer) + scenarios.""" + store = InMemoryProposalStore() + short_ttl = 3600 # 1 hour + platform = _RouterLike( + manager=_AutoCommitManager(ttl_seconds=short_ttl), + store=store, + ) + + before = datetime.now(timezone.utc).timestamp() + await maybe_persist_draft_after_get_products( + platform, + {"products": [], "proposals": [{"proposal_id": "p1"}]}, + _ctx(), + ) + after = datetime.now(timezone.utc).timestamp() + + record = await store.get("p1") + assert record is not None + assert record.expires_at is not None + # expires_at should be ~now + ttl, within the test's wall-clock window. + assert before + short_ttl - 1 <= record.expires_at.timestamp() <= after + short_ttl + 1 + + +@pytest.mark.asyncio +async def test_auto_commit_handles_multiple_proposals_in_one_response() -> None: + """A ``get_products`` response can return multiple proposals; every + one must be promoted independently. No short-circuit on the first.""" + store = InMemoryProposalStore() + platform = _RouterLike(manager=_AutoCommitManager(), store=store) + + await maybe_persist_draft_after_get_products( + platform, + { + "products": [], + "proposals": [ + {"proposal_id": "p1"}, + {"proposal_id": "p2"}, + {"proposal_id": "p3"}, + ], + }, + _ctx(), + ) + + for pid in ("p1", "p2", "p3"): + record = await store.get(pid) + assert record is not None, f"missing proposal {pid}" + assert ( + record.state is ProposalState.COMMITTED + ), f"proposal {pid} expected COMMITTED, got {record.state}" + + +# ----- construction guards (ProposalCapabilities __post_init__) --------- + + +def test_auto_commit_and_finalize_mutually_exclusive() -> None: + """auto-commit and finalize are different lifecycle models. Both + declared simultaneously would race: framework auto-commits, then + buyer's finalize call rejects because the record is no longer + DRAFT. Loud-fail at construction with a clear message.""" + with pytest.raises(AdcpError, match="mutually exclusive"): + ProposalCapabilities( + sales_specialism="sales-non-guaranteed", + finalize=True, + auto_commit_on_put_draft=True, + ) + + +def test_auto_commit_ttl_zero_rejected() -> None: + """Zero or negative TTL would mark proposals expired on commit, + making every consumption attempt fail with PROPOSAL_EXPIRED. + Construction-time loud-fail prevents the misconfig from shipping.""" + with pytest.raises(AdcpError, match="auto_commit_ttl_seconds must be"): + ProposalCapabilities( + sales_specialism="sales-non-guaranteed", + auto_commit_on_put_draft=True, + auto_commit_ttl_seconds=0, + ) + + with pytest.raises(AdcpError, match="auto_commit_ttl_seconds must be"): + ProposalCapabilities( + sales_specialism="sales-non-guaranteed", + auto_commit_on_put_draft=True, + auto_commit_ttl_seconds=-1, + ) + + +def test_auto_commit_default_is_false() -> None: + """Back-compat: existing managers (which don't set the field) keep + the pre-#723 DRAFT-only behavior. Belt-and-suspenders regression + guard so the default isn't accidentally flipped.""" + caps = ProposalCapabilities(sales_specialism="sales-non-guaranteed") + assert caps.auto_commit_on_put_draft is False + + +def test_auto_commit_rejected_on_sales_guaranteed() -> None: + """Product safety guard (raised by review): auto-commit on + ``sales-guaranteed`` issues a silent inventory hold on every + ``get_products`` call. GAM / ad-server proposal lifecycles + require explicit buyer-driven reservation precisely because + trafficking ops won't accept silent holds — a 7-day default TTL + would burn inventory across thousands of catalog probes per day. + Loud-fail with a clear migration path.""" + with pytest.raises(AdcpError, match="sales-guaranteed"): + ProposalCapabilities( + sales_specialism="sales-guaranteed", + auto_commit_on_put_draft=True, + ) + + +def test_auto_commit_long_ttl_emits_soft_cap_warning() -> None: + """A TTL longer than 30 days holds inventory for an entire month + per catalog probe. The framework permits it (long-running RFPs + can legitimately need it) but warns at boot so the choice is + visible. Adopters silence per-process via the warnings filter.""" + import warnings + + with pytest.warns(UserWarning, match="soft cap of"): + ProposalCapabilities( + sales_specialism="sales-non-guaranteed", + auto_commit_on_put_draft=True, + auto_commit_ttl_seconds=45 * 24 * 3600, # 45 days + ) + + # Boundary check: exactly 30 days = no warning (cap is "exceeds", + # not "meets"). + with warnings.catch_warnings(): + warnings.simplefilter("error", UserWarning) + ProposalCapabilities( + sales_specialism="sales-non-guaranteed", + auto_commit_on_put_draft=True, + auto_commit_ttl_seconds=30 * 24 * 3600, # exactly 30 days + ) + + +@pytest.mark.asyncio +async def test_catalog_mode_store_wired_manager_unwired_no_auto_commit() -> None: + """Catalog-mode adopter: ``ProposalStore`` wired but no + ``ProposalManager`` (no proposal-lifecycle dispatch). The + auto-commit branch must be off in this configuration regardless + of what any other manager's capabilities say — we read the + capability off the tenant's own manager, which here is ``None``. + Explicit pin so future refactors that resolve the capability via + a different path (e.g. a router-level default) don't accidentally + enable auto-commit in catalog mode.""" + store = InMemoryProposalStore() + platform = _RouterLike(manager=None, store=store) + + await maybe_persist_draft_after_get_products( + platform, + {"products": [], "proposals": [{"proposal_id": "p1"}]}, + _ctx(), + ) + + record = await store.get("p1") + assert record is not None + assert record.state is ProposalState.DRAFT