fix(decisioning): address review feedback on auto-commit capability (#725)

- Loud-fail ``auto_commit_on_put_draft=True`` on ``sales-guaranteed``
  specialism. Product-expert catch: a 7-day default TTL inventory hold
  issued silently from every ``get_products`` call would burn
  guaranteed-direct inventory across thousands of catalog probes per
  day. GAM / ad-server proposal lifecycles require explicit
  buyer-driven reservation precisely because trafficking ops won't
  accept silent holds. Loud-fail with a clear migration path
  (switch to ``sales-non-guaranteed`` or set ``finalize=True``).

- Implement the >30-day TTL soft-cap warning the docstring promised.
  The framework permits longer TTLs (long-running RFPs legitimately
  need them) but warns at boot so the choice is visible at
  declaration time. Boundary check pins that exactly 30 days does
  not trigger.

- Move ``from datetime import timedelta`` to the module-level import
  (was inline inside the per-proposal loop).

- 3 new tests: ``sales-guaranteed`` rejection, >30d warning + 30d
  boundary, catalog-mode (store wired but manager unwired stays in
  DRAFT regardless).

- Rename local variable ``_SOFT_CAP_SECONDS`` → ``_soft_cap_seconds``
  per ruff N806 (function-local should be lowercase).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Author: bokelley

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 (
@@ -475,8 +475,6 @@ async def maybe_persist_draft_after_get_products(
             # next call's ``try_reserve_consumption`` finds a COMMITTED
             # record. The manager's ``auto_commit_ttl_seconds`` sets
             # the expires_at horizon.
-            from datetime import timedelta
-
             expires_at = datetime.now(timezone.utc) + timedelta(seconds=auto_commit_ttl)
             await _await_maybe(
                 store.commit(

src/adcp/decisioning/proposal_manager.py

@@ -229,6 +229,33 @@ def __post_init__(self) -> None:
                 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",
@@ -242,6 +269,25 @@ def __post_init__(self) -> None:
                 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)

tests/test_proposal_auto_commit.py

@@ -222,3 +222,67 @@ def test_auto_commit_default_is_false() -> None:
     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